-
Notifications
You must be signed in to change notification settings - Fork 771
Update for DynUNet's changes and fix get_feature_maps issue #126
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
Conversation
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 <vennw@nvidia.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
|
@yiheng-wang-nv thanks! Do you think you could add some comments explaining the shape of the tensor returned by |
|
fixes #128. |
|
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 |
|
Hi @coranium this seems like a new issue to me. Could you create a new Discussion, preferably with a minimal working example. |
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
|
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 |
|
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. |
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. |
wyli
left a comment
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.
thanks it looks good to me
|
@yiheng-wang-nv added comments look good, thanks! |
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 vennw@nvidia.com