Skip to content

4587 mri transforms#4591

Merged
wyli merged 82 commits intoProject-MONAI:devfrom
mersad95zd:4587-mri-transforms
Jul 19, 2022
Merged

4587 mri transforms#4591
wyli merged 82 commits intoProject-MONAI:devfrom
mersad95zd:4587-mri-transforms

Conversation

@mersad95zd
Copy link
Copy Markdown
Collaborator

@mersad95zd mersad95zd commented Jun 26, 2022

Fixes # .

Description

MRI transforms as a new feature:

  • mri_transforms.py under monai.apps.reconstruction
  • its unittest under tests
  • its docs under transforms.rst/VanillaTransforms/MRITransforms

Status

Work in progress

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 --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

mersad95zd and others added 22 commits June 21, 2022 07:45
@mersad95zd
mri utils added
e0d20e5 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
fft_utils with its unit test added
aecedaa 
Signed-off-by: mersad95zd <[email protected]>
@Can-Zhao
Merge branch 'dev' into 00-mri-utils
41c93b0
@mersad95zd
fft_utils updated with monai data converter
1e0e39c 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Merge branch '00-mri-utils' of https://github.com/mersad95zd/MONAI in…
34b9da9 
…to 00-mri-utils
@mersad95zd
updated fft_util's docstring
a452ddc 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
apps.rst updated with fft_utils docstrings under the reconstruction m…
962f5f8 
…odule

Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
fft_utils docstring updated by adding dimension hins
829992c 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
fft_utils docstring updated by removing redundant output type
8536cd3 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
test_fft_utils.py moved to the tests folder
ae346ee 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Merge branch 'dev' into 00-mri-utils
41aa0c7
@mersad95zd
Merge branch 'dev' into 00-mri-utils
89f5c99
@mersad95zd
created fft_utils_t, the torch-only version of fft_utils
58086b3 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Merge branch '00-mri-utils' of https://github.com/mersad95zd/MONAI in…
9928973 
…to 00-mri-utils
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
0c3f067 
for more information, see https://pre-commit.ci
@mersad95zd
fft_utils_t updated with type ignore for mypy
f48dc5a 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Merge branch '00-mri-utils' of https://github.com/mersad95zd/MONAI in…
e4773b4 
…to 00-mri-utils
@mersad95zd
docs/source/networks.rst updated with fft_utils_t
7a61979 
Signed-off-by: mersad95zd <[email protected]>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
4fd131f 
for more information, see https://pre-commit.ci
@mersad95zd
manual fix for fft_utils_t output data types
d4ce1fb 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Merge branch '00-mri-utils' of https://github.com/mersad95zd/MONAI in…
eb2b420 
…to 00-mri-utils
@mersad95zd
initial mri_transforms added under apps/reconstruction
4793885 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Copy link
Copy Markdown
Collaborator Author

mersad95zd commented Jun 27, 2022

I'm working on the failed tests, you may review the next commit, thanks!

@mersad95zd mersad95zd marked this pull request as draft July 13, 2022 15:28
rijobro
rijobro reviewed Jul 14, 2022
View reviewed changes
rijobro
rijobro reviewed Jul 14, 2022
View reviewed changes
@rijobro
Copy link
Copy Markdown
Contributor

rijobro commented Jul 14, 2022

RandomKspaceMaskd and EquispacedKspaceMaskd are identical except for self.masker. You could have a parent class that stores all of the functionality and then RandomKspaceMaskd and EquispacedKspaceMaskd only need to create self.masker in the __init__.

mersad95zd and others added 9 commits July 14, 2022 11:29
@mersad95zd
Merge branch 'dev' into 4587-mri-transforms
2a39217
@mersad95zd
added KspaceMaskd and ExtractDataKeyFromMetaKeyd.
25f94a4 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Merge branch '4587-mri-transforms' of https://github.com/mersad95zd/M…
3c7fbba 
…ONAI into 4587-mri-transforms
@mersad95zd
Merge branch 'dev' into 4587-mri-transforms
b3e1d36
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
ca28635 
for more information, see https://pre-commit.ci
@mersad95zd
removed KspaceMaskd and updated RandomKspaceMaskd
ac711dd 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
removed KspaceMaskd and updated RandomKspaceMaskd
6031b9c 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
resolved conflicts in dictionary.py
88985e6 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
added a bit more explanation to fastMRI_reader
be76dd5 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Copy link
Copy Markdown
Collaborator Author

mersad95zd commented Jul 14, 2022

RandomKspaceMaskd and EquispacedKspaceMaskd are identical except for self.masker. You could have a parent class that stores all of the functionality and then RandomKspaceMaskd and EquispacedKspaceMaskd only need to create self.masker in the __init__.

Would it be ok to just create RandomKspaceMaskd, then inherit from it in EquispacedKspaceMaskd?
By doing so, we won't need to define KspaceMaskd.
Another issue with creating KspaceMaskd would be instantiating self.masker in its __init__ because the abstract class KspaceMask can't be instantiated as a self.masker.
So directly defining RandomKspaceMaskd solves this as well.

@mersad95zd
minor docstring fix
39d3d76 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
minor docstring fix
3845a09 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Copy link
Copy Markdown
Collaborator Author

mersad95zd commented Jul 14, 2022

Since this PR already has about 700 lines of code, I'll submit dictionary unit tests in my next PR.

@mersad95zd mersad95zd marked this pull request as ready for review July 14, 2022 23:23
@wyli
Copy link
Copy Markdown
Contributor

wyli commented Jul 19, 2022

/build

@wyli wyli enabled auto-merge (squash) July 19, 2022 11:05
@wyli wyli merged commit 9fb9d98 into Project-MONAI:dev Jul 19, 2022
@mersad95zd mersad95zd deleted the 4587-mri-transforms branch July 27, 2022 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nic-Ma Nic-Ma Nic-Ma left review comments

@rijobro rijobro rijobro left review comments

@Can-Zhao Can-Zhao Can-Zhao approved these changes

@wyli wyli wyli approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mersad95zd @rijobro @Can-Zhao @wyli @Nic-Ma