Skip to content

Conversation

@binliunls
Copy link
Contributor

@binliunls binliunls commented Dec 9, 2022

Signed-off-by: binliu binliu@nvidia.com

Fixes #1090 .

Description

Fix some basic problems occured during test.

Checks

  • Reconstruction tutorial:

    • some typo and grammar problems in the readme file.
    • fastmri_ssim.py absents in varnet_demo folder.
    • the checkpoint device directly map to 'cpu' in unet_demo/inference.ipynb.
  • Detection tutorial:

    • some typo, link and grammar problems in the readme file.
  • CAI tutorial:

    • add explanation for opencv codec error in the video_seg tutorial.
    • use EnsureChannelFirst instead of AsChannelFirst in endoscopic inbody classification tutorial.
    • accuracy in endoscopic inbody classfication tutorial should be a number instead of tensor.
  • Pathology tutorial:

    • some useless cell output in nuclick tutorial.
  • Add license to above tutorials

  • Check license in other tutorials

binliunls and others added 7 commits December 9, 2022 15:28
Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: Bin Liu (SW-GPU) <binliu@nvidia.com>
Signed-off-by: Bin Liu (SW-GPU) <binliu@nvidia.com>
Signed-off-by: Bin Liu (SW-GPU) <binliu@nvidia.com>
…tem to CAI tutorial

Signed-off-by: Bin Liu (SW-GPU) <binliu@nvidia.com>
Signed-off-by: Bin Liu (SW-GPU) <binliu@nvidia.com>
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

binliunls and others added 7 commits December 17, 2022 16:55
Signed-off-by: Bin Liu (SW-GPU) <binliu@nvidia.com>
Signed-off-by: Bin Liu (SW-GPU) <binliu@nvidia.com>
Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: binliu <binliu@nvidia.com>
@binliunls binliunls marked this pull request as ready for review January 5, 2023 03:56
@binliunls binliunls changed the title [WIP]1090-enhance-tutorials 1090-enhance-tutorials Jan 5, 2023
@binliunls binliunls changed the title 1090-enhance-tutorials [WIP]1090-enhance-tutorials Jan 5, 2023
@mingxin-zheng
Copy link
Contributor

Hi @binliunls . Thank you for the PR and making changes on so many files. Below are my comments.

For paired_lung_ct.ipynb:

  • I suggest to re-run the notebook to get rid of the warning message
    UserWarning: This DataLoader will create 4 worker processes in total. Our suggested max number of worker in current system is 2,
    
    /usr/local/lib/python3.7/dist-packages/monai/networks/blocks/warp.py:69: UserWarning: monai.networks.blocks.Warp: Using PyTorch native grid_sample.
    warnings.warn("monai.networks.blocks.Warp: Using PyTorch native grid_sample.")
    
  • The last markdown should be ## Cleanup data directory
  • There is a difference between toolkit and tutorial on the extract_level of the LocalNet

@binliunls
Copy link
Contributor Author

binliunls commented Jan 13, 2023

  • There is a difference between toolkit and tutorial on the extract_level of the LocalNet

Hi @wyli,
I saw you updat this extract_level parameter in the last commit of the paired_lung_ct tutorial. I think we need to update the toolkit with this change. Could you please help me to double check that? cc: @mingxin-zheng

Thanks,
Bin

Signed-off-by: binliu <binliu@nvidia.com>
@wyli
Copy link
Contributor

wyli commented Jan 13, 2023

  • There is a difference between toolkit and tutorial on the extract_level of the LocalNet

Hi @wyli, I saw you updat this extract_level parameter in the last commit of the paired_lung_ct tutorial. I think we need to update the toolkit with this change. Could you please help me to double check that? cc: @mingxin-zheng

sure, in monai v1.1.0 we released a bug fix (Project-MONAI/MONAI@c8ef7fe ), and the fix broke the tutorial (ticket #1056), then I fixed the tutorial with this update (5d490ea).

Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: binliu <binliu@nvidia.com>
@binliunls
Copy link
Contributor Author

In video_seg.ipynb, does the Video work in general settings? @binliunls

Video(inferred_vid, embed=True, height=300)

Hi @mingxin-zheng ,
In all cases I have met, this Video part doesn't work. However, this feature is added by @rijobro . We'd better confirm this with him. If it's not necessary we can delete this part. How do you think about this @rijobro .

Thanks,
Bin

Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: binliu <binliu@nvidia.com>
@rijobro
Copy link
Contributor

rijobro commented Jan 16, 2023

Hi, video doesn't appear to be working for me, either. This stackoverflow post has a few ideas on how to fix if anyone wants to give those a go: https://stackoverflow.com/a/68381044/12411867.

Signed-off-by: Mingxin Zheng <mingxinz@nvidia.com>
@mingxin-zheng
Copy link
Contributor

Hi, video doesn't appear to be working for me, either. This stackoverflow post has a few ideas on how to fix if anyone wants to give those a go: https://stackoverflow.com/a/68381044/12411867.

Hi @rijobro @binliunls , if it needs some tweaking to get it work, how about removing this code cell, or replace it by other solutions, such as writing a video file?

@rijobro
Copy link
Contributor

rijobro commented Jan 16, 2023

I'd be grateful if someone wanted to invest some time to get it working in some capacity. The purpose of the notebook is to segment a video, so it's a shame to not then show the segmented video at the end.

We could save it to file and allow the user to view externally, but this requires more effort from the user, so fewer people will review the video, especially if using Google Colab or jupyter notebook over SSH.

Signed-off-by: Mingxin Zheng <mingxinz@nvidia.com>
@mingxin-zheng
Copy link
Contributor

mingxin-zheng commented Jan 16, 2023

@binliunls did you try to resolve this issue? I remember we had similar conversations a month or two ago about this Video plugin.

@rijobro
We can make this in a new bug fix PR and assign/volunteer ourselves to fix this. One thing that worries me is that we can't keep a code block in the executable area of the notebook when we know it doesn't work for most cases... So I'd prefer to make this a plan in a PR rather than keeping a bug in a notebook....

Signed-off-by: Mingxin Zheng <mingxinz@nvidia.com>
@rijobro
Copy link
Contributor

rijobro commented Jan 16, 2023

I'm definitely ok to have a separate issue/pr to fix the video playback! Don't think it should be deleted from the source code in this pr though - - it'll mess up the diff to delete something and then add it back in.

Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: binliu <binliu@nvidia.com>
…eline

Signed-off-by: binliu <binliu@nvidia.com>
…ideline

Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: binliu <binliu@nvidia.com>
…ttp one to local path

Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: binliu <binliu@nvidia.com>
@mingxin-zheng mingxin-zheng enabled auto-merge (squash) January 17, 2023 07:20
@mingxin-zheng mingxin-zheng merged commit a2652fb into Project-MONAI:main Jan 17, 2023
@binliunls binliunls deleted the 1090-improve-tutorials branch May 4, 2023 08:58
boneseva pushed a commit to boneseva/MONAI-tutorials that referenced this pull request Apr 21, 2024
Signed-off-by: binliu <binliu@nvidia.com>

Fixes Project-MONAI#1090  .

### Description
Fix some basic problems occured during test.

### Checks
- [x] Reconstruction tutorial:
    - [x] some typo and grammar problems in the readme file. 
    - [x] `fastmri_ssim.py` absents in `varnet_demo` folder.
- [x] the checkpoint device directly map to 'cpu' in
[unet_demo/inference.ipynb](https://github.com/binliunls/tutorials/blob/1014-mlflow-handler-tutorial/reconstruction/MRI_reconstruction/unet_demo/inference.ipynb).
   

- [x] Detection tutorial:
     - [x] some typo, link and grammar problems in the readme file. 
  
- [x] CAI tutorial:
- [x] add explanation for opencv codec error in the `video_seg`
tutorial.
- [x] use `EnsureChannelFirst` instead of `AsChannelFirst` in
`endoscopic inbody classification tutorial`.
- [x] accuracy in endoscopic inbody classfication tutorial should be a
number instead of tensor.
   
- [x] Pathology tutorial:
    - [x] some useless cell output in nuclick tutorial.
 
- [x] Add license to above tutorials
- [x] Check license in other tutorials

Signed-off-by: binliu <binliu@nvidia.com>
Signed-off-by: Bin Liu (SW-GPU) <binliu@nvidia.com>
Signed-off-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Signed-off-by: mingxin-zheng <18563433+mingxin-zheng@users.noreply.github.com>
Signed-off-by: Mingxin Zheng <mingxinz@nvidia.com>
Co-authored-by: mingxin-zheng <mingxin-zheng@users.noreply.github.com>
Co-authored-by: Mingxin Zheng <18563433+mingxin-zheng@users.noreply.github.com>
Co-authored-by: Mingxin Zheng <mingxinz@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tutorials bugs to improve

4 participants