Add MONAI Bundle inference operator and update example app #303
Conversation
MMelQin
requested review from
ericspod,
Nic-Ma,
vikashg,
CPBridge and
GreySeaWolf
|
Hey @MMelQin So I am going through the code to test the functions. I think it will be helpful if you can also share any test code that you might have written. For example in and then I have to guess the inputs. I am looking at the spleen_ct_segmentation as the model for testing. |
|
Actually nevermind. I was looking at the diffs and I see the operator being using here I always find the answers after I already post about it 😞 |
| @@ -94,6 +94,7 @@ def compute(self, op_input: InputContext, op_output: OutputContext, context: Exe | |||
| pre_transforms, | |||
| post_transforms, | |||
| overlap=0.6, | |||
| model_name=" ", | |||
There was a problem hiding this comment.
I wonder why model_name has " " instead of "".
There was a problem hiding this comment.
Good catch! I was testing the model names and may have left the blank str. Will make it a empty str, even though the arg default is already empty str.
| return self.known_io_data_types[ctype] | ||
| elif isinstance(ctype, type): # type object | ||
| return ctype | ||
| else: # don't know, something that hasn't been figured out |
There was a problem hiding this comment.
We may want to add some logging message in case this path is executed?
| value = op_input.get(name) | ||
|
|
||
| metadata = None | ||
| if isinstance(value, DataPath): |
There was a problem hiding this comment.
Great to see it is compatible with any storage type!
Just wonder if we were also able to make it work with ordinary image files, instead of pickle files.
For example, If the type of value is DataPath (especially for folder path), instead of removing load operator from bundle's transforms, we may be able to pass the file/folder paths to bundle's load transform?
There was a problem hiding this comment.
Good question! I should have left more comments in the code because I myself thought about this for quite a bit.
- Was considering loading other, and a limited set of, files, e.g. NIfTI/MHD which we already know exactly what to do, but on second thought, decided to clamp down on only pickle file, and will have more discussion on the list of file types to b supported in the future releases (we may end up with reusing the MONAI Core ImageReader impl if need be).
- In my earlier versions of the bundle operator, as well as in the previous release, I actually did customize the LoadImage transform in the pre-process Compose, albeit with the IN_MEM image reader/converter. But now with the bundle API, it makes more sense to treat the loadImage transform opaquely, i.e. either in or out and no need to customize it when
computeis called for a specific payload. The also will allow us to have a more flexibleloadimageimpl in another operator, e.g. I was considering a multi-modal use case where a JSON file is one of the inputs, beside an image file (see the first point).
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
The base inference operator needs to adjust the function signatures due to the expanded use of types resulting from the intro if bundle inference operator. The new version of MyPy also seems to be stricter on types. Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
MMelQin
force-pushed
the
mqin/Issue286_Inference_Operator_for_MONAI_Bundle
branch
from
64ac711 to
17aa29e
Compare
…ts but for consistency. Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
|
Hi @MMelQin, This model-zoo was downloaded from the github repo but I realised that there is no model in it and so I get the following error Can you post a link to the monai bundle you are using, so I can test ? Thanks |
|
@vikashg The large_files.yml for the bundle has the download path for the bundle TorchScript file itself, and once downloaded this file is used for -m arg. MONAI does provide the utility to download a bundle, though I had already done the download before knowing the utility exists. Also, for the bundle operator tutorial Jupyter notebook, I plan to package both the DICOM files AND the bundle TorchScript file in zip shared in GDrive, similar to earlier examples, to make running the example simpler for the users. |
|
Hey @MMelQin, However, I tried to use the model as you said. That is download the torchscript file and then issued the following command I still get this error. I feel I am doing something wrong 😢 Am I ? |
|
Yes with a path to the TorchScript file, which is the portable form of a MONAI Bundle. This TS file can be used as any other TS file, with the added extra data that can be consumed by MONAI Bundle aware/compliant applications or even human users after unzipping the TS. No to path to the Model Zoo URL. An app developer with see the info on Model Zoo, and use the MONAI utility to download the TS file to be used in the app and then MAP. |
|
Okay i will try again tomorrow morning |
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
|
So, I have been trying to make the notebooks work but Still trying to figure out. |
|
Thanks @vikashg I DM'ed you directly on some known issues on setting up App SDK on edit mode with required MONAI core dependency; need to make sure both the core and App SDK modules are under the same monai root that Python interpreter sees. BTW, I've created a placeholder discussion topic and we can collect all the pain points on setting up dev env so as to find ways to smooth out the process and improve dev experience. |
|
Hey @MMelQin Thanks for the detailed message and I will follow along and update you by the end of the date |
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
Signed-off-by: mmelqin <mingmelvinq@nvidia.com>
|
Kudos, SonarCloud Quality Gate passed!
|
|
A discussion topic has been created for improving the developer experience for creating/testing App SDK operators that also requires MONAI core package. When the current process is followed, the setup also works correctly to allow testing the App SDK in edit mode with MONAI Core dependency. |
This PR is based on the bundle_addtion branch that @ericspod created for automating the parsing of MONAI Bundle and execution of inference, with some notable changes: