-
Notifications
You must be signed in to change notification settings - Fork 779
Add new pipeline of DynUNet #132
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>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Signed-off-by: Yiheng Wang <vennw@nvidia.com>
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 |
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, I had a quick look and put some minor comments, I'll have another look. is the notebook repeating the python script's content, shall we remove that?
modules/dynunet_pipeline/inferrer.py
Outdated
| filename, predictions_org.shape, predictions_org.mean() | ||
| ) | ||
| ) | ||
| results_img = nib.Nifti1Image( |
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.
why not using the NiftiSaver from monai?
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.
Hi @wyli , thanks for review. Since I do not use MONAI based resampling method, here using nib directly seems easier. What do you think? Do I need to modify it? In addition, all other places are modified.
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.
yes please try to use 'nifti_writer', we should use monai components as much as possible. the main goal is to verify the monai components and make a high quality demo using monai.
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.
Hi @wyli , I have modified. The old notebook have many duplicate contents, as well as some unreasonable hyperparameters. I removed it in the latest commit.
Co-authored-by: Wenqi Li <wenqil@nvidia.com>
Co-authored-by: Wenqi Li <wenqil@nvidia.com>
Co-authored-by: Wenqi Li <wenqil@nvidia.com>
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!
* Update DynUNet 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 the self-designed loss function part has also been updated. Signed-off-by: Yiheng Wang <vennw@nvidia.com>
Since the old tutorial notebook may has some issues (like the bad performance for anisotropic data, wrong data loading function due to different dataset format and so on), recently I build a new pipeline to improve it. The training scripts for all 10 Decathlon tasks are included.
This PR is used to replace #131 , since it used the same branch as #126 does.