Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MIL component to extract patches #3237

Merged
merged 10 commits into from
Nov 15, 2021
Merged

MIL component to extract patches #3237

merged 10 commits into from
Nov 15, 2021

Conversation

myron
Copy link
Collaborator

@myron myron commented Nov 2, 2021

Fixes #3162

This is a component for MIL to split the large 2D images into patches/tiles in numpy (ndarray views)

Some open questions, where is the best location for it? under apps/pathology ?
There is a somewhat similar named function (SplitOnGrid) #2879 , but it operates on Tensors, and it's for smaller splits (3x3 ?) and has fewer options. But still, people may get confused

@myron myron added this to the Multiple-instance Learning [P0] milestone Nov 2, 2021
@myron myron requested review from wyli, bhashemian and Nic-Ma November 2, 2021 01:01
@myron myron self-assigned this Nov 2, 2021
@bhashemian
Copy link
Member

@myron, this components should go here: https://github.com/Project-MONAI/MONAI/blob/dev/monai/apps/pathology/transforms/spatial/array.py

SplitOnGrid splits the image into patches based on a defined grid for instance 20x20. Although it accepts patch size but the grid is not defined based on that, and allows for cropping and overlapping patches.

I suggest to have something called SplitIntoTiles which defines the grid based on patch size, or extend the SplitOnGrid to include the option for calculating the grid based on a fixed image size.

@myron
Copy link
Collaborator Author

myron commented Nov 2, 2021

I think I'll need a separate transform, since SplitOnGrid is only for pt tensorts, and mine is only for numpy

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 2, 2021

Hi @myron @drbeh ,

Would you like to submit a new transform(array level and dict level)? It's better to make sure:

  1. Both numpy array and PyTorch Tensor data.
  2. Unit tests for all the cases.
  3. Add inverse operation in the dict level transform.
  4. [Optional] Add example transform image in the MONAI API doc, like: https://docs.monai.io/en/latest/transforms.html#spatialpad
    I can help add the example image if you don't have enough effort.

Thanks in advance.

@myron
Copy link
Collaborator Author

myron commented Nov 2, 2021

Hi @myron @drbeh ,

Would you like to submit a new transform(array level and dict level)? It's better to make sure:

1. Both numpy array and PyTorch Tensor data.

2. Unit tests for all the cases.

3. Add **inverse** operation in the `dict` level transform.

4. [Optional] Add example transform image in the MONAI API doc, like: https://docs.monai.io/en/latest/transforms.html#spatialpad
   I can help add the example image if you don't have enough effort.

Thanks in advance.

Hmm, I can't do that much.

  1. it uses a special convenience function in numpy, which doesn't exist for tensors (so it only works in numpy)
  2. unit tests I can do
  3. I'm not sure how to do inverse, it may not be possible (also it is not necessary)
  4. hmm, I'm not sure if we have the rights to publically post these WSI images

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 2, 2021

Hi @myron ,

Sure, please just develop a transform with numpy backend first.
@drbeh or I can work on the following actions.

Thanks.

@myron myron force-pushed the patches branch 2 times, most recently from 2c6c702 to 3d780ab Compare November 2, 2021 23:41
@myron
Copy link
Collaborator Author

myron commented Nov 2, 2021

I've updated this transform, please take a look

Copy link
Member

@bhashemian bhashemian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add unittests and change this PR to a regular PR after that so people can review it? Thanks

monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
@myron
Copy link
Collaborator Author

myron commented Nov 8, 2021

I've updated this transform

  • added option to return a list of dicts
  • added unit tests (all passed)

please review

@myron myron marked this pull request as ready for review November 8, 2021 01:37
tests/test_tile_on_grid.py Outdated Show resolved Hide resolved
@myron
Copy link
Collaborator Author

myron commented Nov 10, 2021

I've made all the corrections, please check

monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
tests/test_tile_on_grid.py Outdated Show resolved Hide resolved
@myron myron force-pushed the patches branch 3 times, most recently from b32617e to ee7f4fe Compare November 10, 2021 20:38
@myron
Copy link
Collaborator Author

myron commented Nov 11, 2021

And BTW, @myron please fix the mypy code format check issues:

monai/apps/pathology/transforms/spatial/array.py:107:27: error: Incompatible default for argument "tile_count" (default has type "None", argument has type "int")  [assignment]
monai/apps/pathology/transforms/spatial/array.py:109:26: error: Incompatible default for argument "tile_step" (default has type "None", argument has type "int")  [assignment]
monai/apps/pathology/transforms/spatial/array.py:131:51: error: Incompatible default for argument "img_size" (default has type "None", argument has type "Sequence[int]")  [assignment]
monai/apps/pathology/transforms/spatial/array.py:139:38: error: Incompatible types in assignment (expression has type "Tuple[int, int]", variable has type "None")  [assignment]
monai/apps/pathology/transforms/spatial/array.py:140:25: error: Value of type "None" is not indexable  [index]
monai/apps/pathology/transforms/spatial/array.py:141:25: error: Value of type "None" is not indexable  [index]
monai/apps/pathology/transforms/spatial/array.py:156:32: error: Incompatible types in assignment (expression has type "ndarray[Any, Any]", variable has type "None")  [assignment]
monai/apps/pathology/transforms/spatial/dictionary.py:88:27: error: Incompatible default for argument "tile_count" (default has type "None", argument has type "int")  [assignment]
monai/apps/pathology/transforms/spatial/dictionary.py:90:26: error: Incompatible default for argument "tile_step" (default has type "None", argument has type "int")  [assignment]
monai/apps/pathology/transforms/spatial/dictionary.py:116:21: error: Incompatible types in assignment (expression has type "int", variable has type "None")  [assignment]
monai/apps/pathology/transforms/spatial/dictionary.py:134:17: error: Incompatible types in assignment (expression has type "List[Dict[Hashable, Any]]", variable has type "Dict[Hashable, ndarray[Any, Any]]")  [assignment]
tests/test_tile_on_grid_dict.py:57:96: error: Incompatible default for argument "tile_filter_mode" (default has type "None", argument has type "str")  [assignment]
tests/test_tile_on_grid_dict.py:80:13: error: Incompatible types in assignment (expression has type "ndarray[Any, Any]", variable has type "List[Any]")  [assignment]
tests/test_tile_on_grid_dict.py:83:34: error: "List[Any]" has no attribute "sum"  [attr-defined]
tests/test_tile_on_grid.py:57:96: error: Incompatible default for argument "tile_filter_mode" (default has type "None", argument has type "str")  [assignment]
tests/test_tile_on_grid.py:80:13: error: Incompatible types in assignment (expression has type "ndarray[Any, Any]", variable has type "List[Any]")  [assignment]
tests/test_tile_on_grid.py:83:34: error: "List[Any]" has no attribute "sum"  [attr-defined]

Thanks.

I've fixed mypy issues

@bhashemian
Copy link
Member

Thank you @myron. Let me reviewed it again before moving forward and please try not to use force-push. Thanks

Signed-off-by: myron <amyronenko@nvidia.com>
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 11, 2021

/black

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 11, 2021

/build

Signed-off-by: myron <amyronenko@nvidia.com>
Copy link
Member

@bhashemian bhashemian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@myron thanks for working this components.I have reviewed this and I left several comments.

Also can you work on removing all # type: ignore. I think that all the situation can be handled without being ignored. Please let me know if you have any question.

*** NOTE *** Please do not force push your next commits and let me know if you are not able to push without it.

monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
monai/apps/pathology/transforms/spatial/array.py Outdated Show resolved Hide resolved
tests/test_tile_on_grid_dict.py Outdated Show resolved Hide resolved
Signed-off-by: myron <amyronenko@nvidia.com>
Signed-off-by: myron <amyronenko@nvidia.com>
tests/test_tile_on_grid.py Outdated Show resolved Hide resolved
tests/test_tile_on_grid_dict.py Outdated Show resolved Hide resolved
myron and others added 2 commits November 14, 2021 22:00
Signed-off-by: myron <amyronenko@nvidia.com>
@bhashemian bhashemian enabled auto-merge (squash) November 15, 2021 15:13
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Nov 15, 2021

/build

@bhashemian bhashemian merged commit 9ec1d14 into Project-MONAI:dev Nov 15, 2021
@myron myron deleted the patches branch January 14, 2022 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split into Tiles
5 participants