Skip to content

Update for DynUNet's changes and fix get_feature_maps issue#126

Merged
wyli merged 5 commits intomasterfrom
106-update-dynunet
Feb 26, 2021
Merged

Update for DynUNet's changes and fix get_feature_maps issue#126
wyli merged 5 commits intomasterfrom
106-update-dynunet

Conversation

@yiheng-wang-nv
Copy link
Copy Markdown
Contributor

@yiheng-wang-nv yiheng-wang-nv commented Feb 22, 2021 •
edited
Loading

Fixes #106 and #128 .

Description
Due to the forward function's changes of DynUNet, this commit
update the corresponding places, as well as the loss calculation
for trainer.

In addition, the DiceCEloss has been implemented in MONAI, thus the
self-designed loss function part has also been updated.

Signed-off-by: Yiheng Wang [email protected]

@yiheng-wang-nv
Update DynUNet
53cb111 
Due to the forward function's changes of DynUNet, this commit
update the corresponding places, as well as the loss calculation
for trainer.

In addition, the DiceCEloss has been implemented in MONAI, thus the
self-designed loss function part has also been updated.

Signed-off-by: Yiheng Wang <[email protected]>
@review-notebook-app
Copy link
Copy Markdown

review-notebook-app bot commented Feb 22, 2021

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@yiheng-wang-nv
Remove unused libraries
d7bff9c 
Signed-off-by: Yiheng Wang <[email protected]>
@yiheng-wang-nv yiheng-wang-nv mentioned this pull request Feb 22, 2021
Merged
6 tasks
rijobro
rijobro reviewed Feb 22, 2021
View reviewed changes
@rijobro
Copy link
Copy Markdown
Contributor

rijobro commented Feb 22, 2021 •
edited by yiheng-wang-nv
Loading

@yiheng-wang-nv thanks! Do you think you could add some comments explaining the shape of the tensor returned by forward? I'm guessing it's the torch.unbind that removes the feature maps? Could you add a comment to that affect, and mention that it's only necessary when deep_supervision==True?

@rijobro
Copy link
Copy Markdown
Contributor

rijobro commented Feb 23, 2021

fixes #128.

@yeeyangtee
Copy link
Copy Markdown

yeeyangtee commented Feb 26, 2021

Hi, somehow am getting labels of shape (2, 1, 40, 56, 40) for taskid = 4.

This causes an error with non-matching dimensions between predictions (4) and targets(5).

I'm guessing onehot_y should be = False and need to squeeze labels. Unless some of the decathlon tasks do not have one-hot-labels

@rijobro
Copy link
Copy Markdown
Contributor

rijobro commented Feb 26, 2021

Hi @coranium this seems like a new issue to me. Could you create a new Discussion, preferably with a minimal working example.

@yiheng-wang-nv
Copy link
Copy Markdown
Contributor Author

yiheng-wang-nv commented Feb 26, 2021

Hi all, I opened a new draft PR, see #131 , since the current tutorial version is too old, which may have many issues, I rewrite a new python file based pipeline, and have tested on all decathlon tasks locally. Since my local version does not have enough doc strings, and used DataParallel rather than DistributedDataParallel for multi-gpu training, to meet the latest MONAI version, the PR is still in processing.
Please take a look at #131 for further updates, thanks!

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Feb 26, 2021

Hi @yiheng-wang-nv do you have an ETA for the new pipeline? if it'll take some time, is it possible to provide a quick fix for #128 first?

@yiheng-wang-nv
Copy link
Copy Markdown
Contributor Author

yiheng-wang-nv commented Feb 26, 2021 •
edited
Loading

Hi @yiheng-wang-nv do you have an ETA for the new pipeline? if it'll take some time, is it possible to provide a quick fix for #128 first?

I see, let me reopen this PR, and create another one to distinguish the new pipeline. The new pipeline will be finished next week.

@yiheng-wang-nv
Copy link
Copy Markdown
Contributor Author

yiheng-wang-nv commented Feb 26, 2021

@yiheng-wang-nv thanks! Do you think you could add some comments explaining the shape of the tensor returned by forward? I'm guessing it's the torch.unbind that removes the feature maps? Could you add a comment to that affect, and mention that it's only necessary when deep_supervision==True?

Hi @rijobro , I've added the comments, could you please help to review it again? As for the new pipeline I mentioned above, I'll open another branch and submit a PR after finishing it.

@yiheng-wang-nv yiheng-wang-nv changed the title Update DynUNet Update for DynUNet's changes and fix get_feature_maps issue Feb 26, 2021
@yiheng-wang-nv yiheng-wang-nv mentioned this pull request Feb 26, 2021
Closed
wyli
wyli approved these changes Feb 26, 2021
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

@yiheng-wang-nv yiheng-wang-nv mentioned this pull request Feb 26, 2021
Merged
@rijobro
Copy link
Copy Markdown
Contributor

rijobro commented Feb 26, 2021

@yiheng-wang-nv added comments look good, thanks!

@wyli wyli merged commit b670c8d into master Feb 26, 2021
@wyli wyli deleted the 106-update-dynunet branch April 20, 2021 14:17
boneseva pushed a commit to boneseva/MONAI-tutorials that referenced this pull request Apr 21, 2024
@yiheng-wang-nv
Update for DynUNet's changes and fix get_feature_maps issue (Project-…
56845ec 
…MONAI#126)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rijobro rijobro rijobro left review comments

@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.

DynUNet needs updating for deep_supervision

4 participants

@yiheng-wang-nv @rijobro @yeeyangtee @wyli