-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[2678] Add transform to fill holes and to filter #2692
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
Conversation
1f436ed to
29b3b18
Compare
a42774a to
f0f97d0
Compare
|
Hi @Spenhouet , Thanks for your quick PR.
Thanks. |
f0f97d0 to
110d16b
Compare
|
@Nic-Ma I don't know how to address the type hint regarding numpy. This does not seem to have been done anywhere in the repository so far. I did use the NdarrayTensor type which is already in use. This one is also not further typeable. So I really don't know how to resolve this? |
|
Hi @Spenhouet , Maybe you can change that line to: return np.asarray(np.where(np.isin(img, self.applied_labels), img, 0))Thanks. |
|
@Nic-Ma This resolves it for mypy but then Pylance complains:
But since you are not linting with Pylance... this would resolve the issue I guess. Feels just like this actually a mypy issue and should be reported as bug on their side. |
110d16b to
b650ab9
Compare
|
@Nic-Ma @ericspod I feel like the discussion on #2678 is not finished yet. As I see it, the pull request should rather implement the |
wyli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
@wyli How about |
|
Labelfilter sounds good to me |
ffb52cb to
7174df8
Compare
|
I now changed the implementation to the "growing" logic. It is way faster than the connected component one and it becomes much faster if the filling is only done for a subset of labels. I implemented it so that it directly works on batches and does not need to iterate over batches. The pull request should be reviewable now. |
21dd413 to
3657124
Compare
|
@Nic-Ma Something failed here but I can't see the logs of the pull request checks. |
|
We should merge this version first once we've figured out the issue with the tests. I can't see the logs either so @Spenhouet if you update the branch with changes from dev this will trigger the tests again and hopefully you'll get an error log this time. The solution later down the line is to define a convolution operation using max rather than sum as its function or otherwise define binary morphology operators in a Pytorch-compatible way on GPU. |
|
there was an issue with the github action today https://www.githubstatus.com/incidents/7p1nnvkgh96y let me rerun this pipeline |
|
@wyli Thanks for rerunning the pipeline. The checks passed now. |
Signed-off-by: Sebastian Penhouet <sebastian.penhouet@airamed.de>
Signed-off-by: Sebastian Penhouet <sebastian.penhouet@airamed.de>
Signed-off-by: Sebastian Penhouet <sebastian.penhouet@airamed.de>
Signed-off-by: Sebastian Penhouet <sebastian.penhouet@airamed.de>
8a60e3a to
13f759a
Compare
Signed-off-by: Sebastian Penhouet <sebastian.penhouet@airamed.de>
|
Interestingly the performance for one-hot encoded images is way better when iterating over the encoding instead of letting the Implementation with windows / structure: structure = np.zeros((3, *[3] * spatial_dims))
structure[1, ...] = ndimage.generate_binary_structure(spatial_dims, connectivity or spatial_dims)
img_arr_filtered = img_arr[tuple(applied_labels), ...]
tmp = np.zeros(img_arr_filtered.shape, dtype=bool)
ndimage.binary_dilation(
tmp,
structure=structure,
iterations=-1,
mask=np.logical_not(img_arr_filtered),
origin=0,
border_value=1,
output=tmp,
)
img_arr[tuple(applied_labels), ...] = np.logical_not(tmp)Implementation with iteration: structure = ndimage.generate_binary_structure(spatial_dims, connectivity or spatial_dims)
for label in applied_labels:
tmp = np.zeros(img_arr.shape[1:], dtype=bool)
ndimage.binary_dilation(
tmp,
structure=None,
iterations=-1,
mask=np.logical_not(img_arr[label]),
origin=0,
border_value=1,
output=tmp,
)
img_arr[label] = np.logical_not(tmp)
I will therefore implement the iteration. |
Signed-off-by: Sebastian Penhouet <sebastian.penhouet@airamed.de>
|
I pushed the change and now let's see if the checks pass. |
|
/black |
Nic-Ma
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me now.
Thanks so much for the great feature and implementation!
Signed-off-by: monai-bot <monai.miccai2019@gmail.com>
Fixes #2678 .
Description
A few sentences describing the changes proposed in this pull request.
Status
Ready/Work in progress/Hold
Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests.make htmlcommand in thedocs/folder.