Skip to content

Rician Noise Transform #2066

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

Merged
merged 10 commits into from
Apr 29, 2021
Merged

Conversation

lyndonboone
Copy link
Contributor

@lyndonboone lyndonboone commented Apr 21, 2021

Description

Added a Rician noise transform for modeling noise in MRI.

closes #2054

Status

ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@wyli
Copy link
Contributor

wyli commented Apr 21, 2021

thanks, it looks nice! just a few minor things to improve:

@lyndonboone
Copy link
Contributor Author

@wyli thanks for the comments! I added a couple of unit tests that closely resemble tests/test_rand_gaussian_noise.py and tests/test_rand_gaussian_noised.py.

These test the default options for the Rician noise transform, which make it act the same as the current Gaussian noise transform. I also added a couple of additional options to the Rician noise transform:

  • relative allows the std to be specified relative to the image intensity histogram's standard deviation.
  • channel_wise allows the above to be done channel-wise, or with different values per channel (mechanics based on the NormalizeIntensity transform).
  • sample_std toggles whether the std of the applied noise is sampled from a uniform distribution (as in the current Gaussian noise transform) or is left as specified. The first case would be typical during training/augmentation but the second case may be useful if the user would like the same degree of noise to be applied each time for some reason (e.g. perhaps doing some experiments at test time).

Whenever you get a chance, let me know what you think of these options and whether I should include unit tests for them. I'm fairly new to writing unit tests but I'll do my best to figure it out if necessary.

I'll also try to take a look at the flake8 issue over the weekend or early next week.

@wyli
Copy link
Contributor

wyli commented Apr 27, 2021

the PR looks good to me, just need to fix the flake8 issue, and keep the branch uptodate with the master branch then I'll merge it. Many thanks!

@lyndonboone
Copy link
Contributor Author

@wyli thanks! When I run the mypy tests on my local machine I get the error below, which is preventing me from getting to the typing errors. Any ideas on how I might be able to get around this?
image

@rijobro
Copy link
Contributor

rijobro commented Apr 28, 2021

What OS are you on?

@lyndonboone
Copy link
Contributor Author

Thanks for the reply @rijobro. I'm using WSL2

Signed-off-by: Lyndon Boone <[email protected]>
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

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

the PR looks good to me, thank you!

@wyli wyli changed the title [WIP] Rician Noise Transform Rician Noise Transform Apr 28, 2021
@wyli wyli enabled auto-merge (squash) April 28, 2021 23:51
@lyndonboone
Copy link
Contributor Author

No problem, thanks @wyli !

@rijobro the problem I was having seems to be related to this issue in mypy: python/mypy#7281. I temporarily moved the BlockArgs class definition to outside the _decode_block_list function in efficientnet.py which allowed me to get at the typing errors.

@wyli wyli merged commit ca26e51 into Project-MONAI:dev Apr 29, 2021
@lyndonboone lyndonboone deleted the 2054-rician-noise-transform branch April 30, 2021 02:09
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.

Rician Noise Transform
3 participants