-
Notifications
You must be signed in to change notification settings - Fork 696
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
Call Vista3D NIM on NVIDIA AI enterprise / GPU cloud #1898
base: main
Are you sure you want to change the base?
Conversation
Illustrates functions for uploading images to file.io and using their URLs to launch Vista3D NIM on NVIDIA AI enterprise / GPU cloud servers. Enables researchers with limited GPU resources to take advantage of NVIDIA resources while evaluating MONAI Vista3D.
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
for more information, see https://pre-commit.ci
I, Stephen Aylward <[email protected]>, hereby add my Signed-off-by to this commit: 8579ce1 Signed-off-by: Stephen Aylward <[email protected]>
I, Stephen Aylward <[email protected]>, hereby add my Signed-off-by to this commit: 9355c8a Signed-off-by: Stephen Aylward <[email protected]>
I, Stephen Aylward <[email protected]>, hereby add my Signed-off-by to this commit: 9a7dd82 Signed-off-by: Stephen Aylward <[email protected]>
I, Stephen Aylward <[email protected]>, hereby add my Signed-off-by to this commit: 667f525 Signed-off-by: Stephen Aylward <[email protected]>
@@ -0,0 +1,510 @@ | |||
{ |
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.
Do you think adding two screenshots here would improve the overall clarity and visual appeal?
Reply via ReviewNB
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.
Added.
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.
You may need using ![Figure](Figure.png)
, the screenshots not shown as expected here: https://github.com/Project-MONAI/tutorials/blob/95eff84dfbadcfba066c3b09e88176d905e1aff7/nvidia_nims/vista_3d_remote_nim.ipynb
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 for the PR! Looks good to me.
Put two minor comments.
Signed-off-by: Stephen Aylward <[email protected]>
@@ -0,0 +1,523 @@ | |||
{ |
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.
Line #5. invoke_url = "https://health.api.nvidia.com/v1/medicalimaging/nvidia/vista-3d"
I would turn this into an argument for the function, and where you call this function pass in this URL explaining what it is, and maybe suggest other models that would work here too to do something different. I think that helps illustrate what the end point is in the notebook by explicitly using it in a cell rather than being hard coded in a function. At the top of the notebook I would also provide very brief descriptions of what VISTA 3D and NIM are, but very brief next to the URLs you provide for each.
Reply via ReviewNB
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.
Quite a bit of this function is specific to the VISTA 3D NIM and this web interface/URL (e.g., the validation logic and the URL request parameters/arguments). So, I don't think the URL should be a parameter to this function.
However, I agree that a brief description of VISTA 3D and NIMs are needed, and I will add them.
@@ -0,0 +1,523 @@ | |||
{ |
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.
Line #9. isotropy = (
Here I'd recommend saving the spacing to a variable: input_spacing = nput_image.GetSpacing()
and then use that variable everywhere you're calling input_image.GetSpacing()
. This reduces the code a bit and helps with readability.
Reply via ReviewNB
@@ -0,0 +1,523 @@ | |||
{ |
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.
Line #55. return itk.imread(filepath, pixel_type=itk.SS)
if imread
can read from a data stream you can use z.open
to open streams to files in the zip file without extracting it, for large zips this can be faster.
Reply via ReviewNB
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.
Streaming from zips is already done by ITK (i.e., imread) depending on the file format (e.g., you can stream MetaIO files). It also has a very fast way of parsing directories of DICOM objects to load 3D volumes (by default loading the first series in the directory, if a series ID isn't specified).
I am not certain if ITK provides a way of specifying a collection of DICOM via gzip - I know we have written programs on top of ITK that do that, but interesting idea to add that capability to ITK's imread directly. As you note, it would be easy. It seems outside the scope of this notebook, or am I missing something?
@@ -0,0 +1,523 @@ | |||
{ |
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.
Line #1. ngc_api_key = os.environ.get("NGC_API_KEY")
It may be worth noting here that people can just paste their API keys here if they're in an environment that setting an variable isn't feasible.
Reply via ReviewNB
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.
Ok - I am willing to add that, as long as we also note that it is bad practice :). Too often people forget to delete the keys before checking in their code, thereby inadvertently advertising their keys. We shouldn't encourage that practice, imo.
I had some super minor comments on the notebook but otherwise looks good to me. We can't see the PyVista widgets in the notebook when rendered in Github, I wonder if adding the screenshots in their place as dummies would work. What you could do is run a rendering cell with the code commented and instead displaying an image, then restore the content of the cell with the code before saving the notebook. |
Fixes # .
Description
Illustrates functions for uploading images to file.io and using their URLs to launch Vista3D NIM on NVIDIA AI enterprise / GPU cloud servers.
Enables researchers with limited GPU resources to take advantage of NVIDIA resources while evaluating MONAI Vista3D.
Checks
./figure
folder./runner.sh -t <path to .ipynb file>