-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[Feature] nnUNet-style Gaussian Noise and Blur #2373
Conversation
Codecov ReportBase: 83.57% // Head: 83.61% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## dev-1.x #2373 +/- ##
===========================================
+ Coverage 83.57% 83.61% +0.04%
===========================================
Files 144 144
Lines 8238 8302 +64
Branches 1226 1237 +11
===========================================
+ Hits 6885 6942 +57
- Misses 1160 1161 +1
- Partials 193 199 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Hi @blueyo0, |
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.
please add some ut for BioMedicalGaussianBlur
4c28e1e
to
115552d
Compare
# if `self.different_sigma_per_channel` is True, | ||
# re-generate random sigma for each channel | ||
if (sigma is None or self.different_sigma_per_channel): | ||
if (not self.different_sigma_per_axis): |
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.
From this implementation, only when self.different_sigma_per_axis
is True, it will generate different sigma for each channels along Z axis?
However, different_sigma_per_axis
is not working for the function described in docstring, it works as different_sigma_per_channel
different_sigma_per_axis (bool): whether to use different
sigma for axis X and Y of the image. Default to True.
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.
Here are some mistakes in this docstring. Actually, different_sigma_per_axis
controls axis X, Y, Z, and different_sigma_per_channel
controls axis N. The docstring is corrected now.
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.
There is a little question at L1657-1659, If different_sigma_per_axis
is True, it will generate different sigma along Z? not X Y Z?
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.
sorry I got it, it generates sigma for data_sample.shape[1:]
, I just ignore :
c11ad1e
to
8452713
Compare
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.
LGTM
* add UniPC scheduler * add the return type to the functions * code quality check * add tests * finish docs --------- Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>
## Motivation implement nnUNet-style Gaussian Noise and Blur
Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.
Motivation
implement nnUNet-style Gaussian Noise and Blur
Modification
Please briefly describe what modification is made in this PR.
BC-breaking (Optional)
Does the modification introduce changes that break the backward-compatibility of the downstream repos?
If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
Use cases (Optional)
If this PR introduces a new feature, it is better to list some use cases here, and update the documentation.
Checklist