Skip to content

e2e-varnet#4853

Merged
wyli merged 20 commits intoProject-MONAI:devfrom
mersad95zd:4673-e2e-varnet
Aug 12, 2022
Merged

e2e-varnet#4853
wyli merged 20 commits intoProject-MONAI:devfrom
mersad95zd:4673-e2e-varnet

Conversation

@mersad95zd
Copy link
Copy Markdown
Collaborator

@mersad95zd mersad95zd commented Aug 8, 2022 •
edited
Loading

Signed-off-by: mersad95zd [email protected]

Fixes # .

Description

This pull request intends to combine ComplexUNet, CoilSensitivityModel, and VarNetBlock (all are now monai modules) to create the e2e-VarNet model.

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 10 commits August 8, 2022 00:54
@mersad95zd
initial commit of e2e-varnet
4b61d01 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Merge branch 'dev' into 4673-e2e-varnet
bc783b4
@mersad95zd
initial commit of e2e-varnet
ead2955 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Merge branch '4673-e2e-varnet' of https://github.com/mersad95zd/MONAI …
b6dcf21 
…into 4673-e2e-varnet
@mersad95zd
docstrings updated; model attributes changed and polished according t…
e3e535a 
…o monai style

Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
added docs
e9e7be9 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
Merge branch 'dev' into 4673-e2e-varnet
9d9c9fc
@mersad95zd
Merge branch '4673-e2e-varnet' of https://github.com/mersad95zd/MONAI …
477a6a0 
…into 4673-e2e-varnet
@mersad95zd
reformatted varnet
f1c783f 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd
fixed varnet forward
a7ccd70 
Signed-off-by: mersad95zd <[email protected]>
@mersad95zd mersad95zd marked this pull request as ready for review August 8, 2022 19:19
@mersad95zd mersad95zd requested review from Can-Zhao and wyli August 8, 2022 19:19
@mersad95zd
Copy link
Copy Markdown
Collaborator Author

mersad95zd commented Aug 8, 2022

I also had model parallelism in mind to add, but will do that as an extra feature in a future PR.

@Can-Zhao
Copy link
Copy Markdown
Collaborator

Can-Zhao commented Aug 9, 2022

Looks good. Could you add a unit test? Thanks!

@mersad95zd mersad95zd marked this pull request as draft August 11, 2022 02:22
mersad95zd
mersad95zd commented Aug 11, 2022
View reviewed changes
mersad95zd
mersad95zd commented Aug 11, 2022
View reviewed changes
mersad95zd
mersad95zd commented Aug 11, 2022
View reviewed changes
mersad95zd
mersad95zd commented Aug 11, 2022
View reviewed changes
@mersad95zd
Copy link
Copy Markdown
Collaborator Author

mersad95zd commented Aug 11, 2022

This PR grew in size from my initial commit, apologies for that. To make the model jit script compatible, I had to make adjustments to several modules that I wrote before.

New elements are only few and I've marked those in the comments above. The rest of the codes are either minor reformatting or Tensor version of some functions denoted by _t for jit compatibility.

Please let me know if I should do anything to make this easier to review, thanks!

@mersad95zd mersad95zd marked this pull request as ready for review August 11, 2022 07:05
wyli
wyli reviewed Aug 12, 2022
View reviewed changes
Copy link
Copy Markdown
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.

thanks, I put some minor comments inline

@mersad95zd
Copy link
Copy Markdown
Collaborator Author

mersad95zd commented Aug 12, 2022

Thanks @wyli... I completely forgot about removing the redundancy between _t and ordinary functions... Now each function calls its script _t function.

wyli
wyli approved these changes Aug 12, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Thanks, it looks good to me, I'll merge this PR if all tests work fine.

@mersad95zd
Copy link
Copy Markdown
Collaborator Author

mersad95zd commented Aug 12, 2022

Thank you!

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Aug 12, 2022

/build

@wyli wyli enabled auto-merge (squash) August 12, 2022 10:15
@wyli wyli merged commit d77b327 into Project-MONAI:dev Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wyli wyli wyli approved these changes

@Can-Zhao Can-Zhao Awaiting requested review from Can-Zhao

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mersad95zd @Can-Zhao @wyli