Repository navigation
Add cucim.skimage.morphology.medial_axis - #342
Merged
Merged
Conversation
fix automated setting of m1, m2, m3 parameters for 2D PBA+
When switching to 64-bit int can go up to size 2**20 on each axis.
grlee77
added
feature request
Contributor
Author
|
Acceleration is around 10x relative to scikit-image v0.19.3. That is pretty good considering that there is still a sequential portion that needs to run on the GPU. |
rapids-bot Bot
pushed a commit
that referenced
this pull request
Jul 28, 2022
### Overview closes #319 This PR adds an implementation of `scipy.ndimage.distance_transform_edt`. For each foreground pixel in a binary image, this function computes the minimal Euclidean distance to reach a background pixel. This is a SciPy function rather than a scikit-image one so I have put it under `cucim.core` instead of `cucim.skimage`. Longer term we could move this upstream to CuPy, but I think we want to have a copy here so that we can use it in the near term to implement some of the missing scikit-image functionality. This function is used by the following functions in the scikit-image API, so this PR will help us implement these in future PRs: `skimage.morphology.medial_axis` (**update**: see #342) `skimage.segmentation.expand_labels` (**update**: see #341) `skimage.segmentation.chan_vese` (**update**: see #343) It is also used in examples related to `skimage.segmentation.watershed` and is needed for implementation of ITK's `SignedMaurerDistanceMapImageFilter` for [itk-cucim](InsightSoftwareConsortium/itk_cucim#13). The algorithm used here is adapted from the C++ code in the [PBA+ repository](https://github.com/orzzzjq/Parallel-Banding-Algorithm-plus) (MIT-licensed) ### extensions made to PBA+ kernel source - larger sizes can be supported due to runtime code generation of the integer datatypes and encode/decode functions. ### current known limitations - 2D and 3D only (this should cover the most common use cases) - The `sampling` argument is not fully supported. This can likely be done with a bit of effort, but will require modifications to the CUDA kernels - User-specified output arguments are not currently supported. We can potentially add this in the future - The indices returned by `return_indices=True` are equally valid, but not always identical to those returned by SciPy. This is because there can be multiple indices with an identical distance, so which one gets chosen in that case is implementation-dependent. ### initial benchmarks relative to SciPy Here `% true` is the percentage of the image that corresponds to foreground pixels. Even for fairly small images there is some benefit, with the benefit becoming two orders of magnitude at larger sizes. shape | % true | cuCIM | SciPy | acceleration -----------------|--------|---------|---------|-------------- (256, 256) | 5 | 0.00108 | 0.00353 | 3.25 (512, 512) | 5 | 0.00108 | 0.01552 | 14.42 (2048, 2048) | 5 | 0.00252 | 0.34434 | 136.86 (4096, 4096) | 5 | 0.00765 | 1.58948 | 207.88 (32, 32, 32) | 5 | 0.00103 | 0.00305 | 2.98 (128, 128, 128) | 5 | 0.00153 | 0.30103 | 196.26 (256, 256, 256) | 5 | 0.00763 | 3.17872 | 416.37 (384, 384, 384) | 5 | 0.02460 | 14.28779 | 580.89 (256, 256) | 50 | 0.00107 | 0.00430 | 4.01 (512, 512) | 50 | 0.00109 | 0.01878 | 17.30 (2048, 2048) | 50 | 0.00299 | 0.39304 | 131.60 (4096, 4096) | 50 | 0.00896 | 1.84686 | 206.19 (32, 32, 32) | 50 | 0.00102 | 0.00361 | 3.53 (128, 128, 128) | 50 | 0.00163 | 0.31657 | 194.66 (256, 256, 256) | 50 | 0.00914 | 3.35914 | 367.49 (384, 384, 384) | 50 | 0.03005 | 13.83219 | 460.30 (256, 256) | 95 | 0.00107 | 0.00344 | 3.22 (512, 512) | 95 | 0.00108 | 0.01638 | 15.17 (2048, 2048) | 95 | 0.00314 | 0.36996 | 117.81 (4096, 4096) | 95 | 0.00943 | 1.90475 | 202.05 (32, 32, 32) | 95 | 0.00102 | 0.00314 | 3.07 (128, 128, 128) | 95 | 0.00180 | 0.28843 | 159.80 (256, 256, 256) | 95 | 0.01073 | 3.23450 | 301.41 (384, 384, 384) | 95 | 0.03577 | 12.40526 | 346.82 ### other comments - can likely reduce memory overhead and improve performance a bit by refactoring some of the pre/post-processing code into elementwise kernels. (e.g. `encode3d`/`decode3d`, etc.) - (JK) This may be able to leverage CuPy in the future ( cupy/cupy#6919 ) Authors: - Gregory Lee (https://github.com/grlee77) Approvers: - Gigon Bae (https://github.com/gigony) - https://github.com/jakirkham URL: #318
Member
Contributor
Author
|
fixed the conflict |
jakirkham
approved these changes
Aug 1, 2022
Member
|
@gigony, could you please review this as well? 🙂 |
gigony
approved these changes
Aug 2, 2022
|
|
||
| class TestMedialAxis(): | ||
| def test_00_00_zeros(self): | ||
| '''Test skeletonize on an array of all zeros''' |
Contributor
There was a problem hiding this comment.
nit: just wondering if there is a reason to use ''' instead of """ in this file.
Member
There was a problem hiding this comment.
Yeah we should probably move to """. This is what black does
Contributor
Author
There was a problem hiding this comment.
Happy to change this to """ and we can probably apply black at some point, but I didn't do it so far for two reasons
1.) It messes with readability of various small test array definitions unless # fmt: off, etc. are placed around them
2.) easier to compare diffs with upstream scikit-image without it.
Member
|
Fixed a small conflict above. If that looks ok, please feel free to merge 🙂 |
Contributor
Author
|
@gpucibot merge |
Contributor
Author
|
@jakirkham: I tried to merge via gpucibot in my previous comment, but it didn't seem to listen to me. Do I need to be granted some permissions here for it to work? |
Contributor
|
@gpucibot merge |
Member
|
Thanks Ray! 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #336
This PR adds a function for skeletonization of 2D images via the medial axis transform.
It should be reviewed after #318 is merged. The new commits only start from 19a6fed.
There is one sequential component to this algorithm that still must be run on the CPU, but the majority of the computations are on the GPU and acceleration is good. I will add benchmark results here soon.