Skip to content
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

Develop MVP of model bundle #3482

Closed
15 of 17 tasks
Nic-Ma opened this issue Dec 13, 2021 · 38 comments
Closed
15 of 17 tasks

Develop MVP of model bundle #3482

Nic-Ma opened this issue Dec 13, 2021 · 38 comments

Comments

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Dec 13, 2021

Is your feature request related to a problem? Please describe.
Thanks for the interesting technical discussion with @ericspod @wyli @atbenmurray @rijobro , as we still have many unclear requirements and unknown use cases, we plan to develop the model package feature step by step, May adjust the design based on feedback during the development.

For the initial step, the core team aligned to develop a small but typical example for inference first, it will use JSON config files to define environments, components and workflow, save the config and model into TorchScript model. then other projects can easily reconstruct the exact same python program and parameters to reproduce the inference. When the small MVP is ready, will share and discuss within the team for the next steps.

I will try to implement the MVP referring to some existing solutions, like NVIDIA Clara MMAR, ignite online package, etc. Basic task steps:

@Nic-Ma Nic-Ma self-assigned this Dec 13, 2021
@Nic-Ma Nic-Ma added this to MONAI 0.9 Dec 13, 2021
@dbericat
Copy link
Member

@CPBridge this is the current PR to expand model metadata.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Dec 14, 2021

Hi @wyli @ericspod @rijobro @atbenmurray ,

I updated the ticket description to make it more clear.

Thanks.

@wyli
Copy link
Contributor

wyli commented Dec 14, 2021

We've discussed that the MVP should focus on the minimal structure, including model weights, basic environment/system info, maintaining changelog/versioning, how to ensure accessibility for both human and machine.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Dec 14, 2021

Hi @wyli ,

Sure, I added these items to the MVP task list.

Thanks.

@dbericat
Copy link
Member

@MMelQin @gigony @GreySeaWolf @CPBridge @mocsharp @joshliberty @vikashg thoughts on proposed MVP?

@ericspod
Copy link
Member

We should discuss a clear definition of requirements and objectives. We want to define a format of a single file or multiples files which contains the model weights at least with secondary information describing how to use it for various use cases. This will allow a human or a program to determine what sort of model it is, how to use the model, and what tasks to use it for. For our MVP we want to consider the starting position of what the model weight storage and metadata storage would look like and if it would achieve that objective to some degree.

The base level set of requirements I would suggest are:

  • Ability to save model weights
  • Ability to save runnable model in Torchscript and/or ONNX
  • Include information on model name, version, description, authorship, intended use
  • Include package and technical information needed to use model, eg. MONAI version, Pytorch and other package versions
  • Description of input and output, eg. how to interpret predictions returned
  • Description in some form of how to transform input data into the form expected by the model (ie. preprocess transform sequence) and transform for output

One use case for this information is a human user looking into how the model is used in a particular task. They would want a clear idea of what inputs are expected and what the outputs mean. Whatever format this information is can be either easily read by a human or easily converted into a convenient format using included tools.

A second use case is a deployment environment which automatically constructs whatever infrastructure is needed around a model to present it through a standard interface. This would require generating transform sequences automatically to pre- and post-process data, and load the model through some script defining the workflow. This would used by MONAI Deploy to automatically generate a MAP from the package, or another script automatically create a Docker image to serve the model as a command line tool or through Flask, or another script to interface with existing hosting services to upload the model and whatever other information is needed.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Dec 15, 2021

Hi @ericspod ,

Thanks very much for your detailed description.
I will try to prepare a draft model package for discussion according to your summary and our basic ideas, and then develop the necessary features to support it.

Thanks.

@wyli
Copy link
Contributor

wyli commented Dec 16, 2021

I'd strongly recommend that the MVP should support the essential features for fast prototyping of model packages. The following uses the popular detectron2 package as an example:

These are mainly to ensure a good flexibility and extensibility of the packaging.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Dec 16, 2021

Hi @wyli ,

Thanks for your great suggestions.
These are very good references for config parsing logic in steps 3 and 4 of this ticket, we definitely need similar features for flexibility and extensibility.

Thanks.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Dec 16, 2021

Hi @wyli @ericspod @atbenmurray @rijobro ,

I tried to prepare a draft model package structure for the first step of this ticket in the tutorial PR.
Project-MONAI/tutorials#487
Could you please help take a look and share some feedback for the model package structure?
(1) I didn't implement any config-parsing related logic so far, I think that's later steps.
(2) I am not very sure to put some description strings in metadata.json or README.md, so I put in both sides, would like to see your ideas.

Thanks in advance.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Dec 23, 2021

Hi @ericspod ,

Thanks for your great summary!
There is another related PR: #3518.

Thanks.

@wyli
Copy link
Contributor

wyli commented Jan 10, 2022

for the record, we could consider this cache dataset config vs content Integrity issue #999.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jan 17, 2022

Hi @ericspod @wyli @dbericat ,

Another thing I want to mention is that: we can also consider to add the package format verification with predefined schemes, for example: check the JSON syntax, check necessary fields in the meta data, check essential folders, etc.

Thanks.

@ericspod
Copy link
Member

Hi @ericspod @wyli @dbericat ,

Another thing I want to mention is that: we can also consider to add the package format verification with predefined schemes, for example: check the JSON syntax, check necessary fields in the meta data, check essential folders, etc.

Thanks.

We'll certainly want to check the format of specific fields in the json data so that they correctly adhere to a standard way of describing transforms and inference so that code can be generated from them. With the other data that can be added it would be preferrable that this can be as free-form as people like, so I would expect standardized elements in the metadat adhering to our format mixed in with ones that are application-specific in any format.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jan 17, 2022

@ericspod sounds good to me! We can do that when we finalized the first MVP MMAR example.

Thanks.

@MMelQin
Copy link

MMelQin commented Jan 21, 2022

As for the machine consumer of the model package, specifically TrochScript, we do have a specific one, Triton Inference Server. Triton can automatically generate runtime config for all the backends it supports, except PyTorch. Triton team had logged an issue for PyTorch, though based on the discussions we've already had here, the requested metadata are covered in the MONAI model package. Here is the PyTorch issue 38273,

@MMelQin
Copy link

MMelQin commented Jan 21, 2022

For deployment of AI apps/models in clinical environment, there are a number of persona and user stories.

  • Reporting of the AI result needs certain metadata of the model (some of them are already covered in proposal) per IHE AIR R1.1. Model information needs to be identified, per 6.5.3.1 General Result Encoding Requirements, more specifically,
  1. Purpose of Reference Code Sequence (0040,A170) shall be (Newcode1, 99IHE,1630 "Processing Algorithm")
  2. Manufacturer (model creator/trainer)
  3. Manufacturer’s Model Name (model name)
  4. Software Versions (version)
  5. Device UID (UID for the model. Each time an AI Model is modified, for example by training, it would be appropriate to update the Device UID.)
  • For integration with DICOM network and selection of relevant series to feed the model, an application needs some metadata description about the DCIOM study/series,
  1. Modality Type (DICOM standards has a defined set
  2. Minimum resolution of images, expressed in max pixel spacings (nice to have; it has been used in practice)
  3. Demographic info, e.g. minimal age of patient the model can be applied to (nice to have)
  • For models with multi-channel, e.g. Brain tumor segmentation model taking in images with T1, T2, FLARE, etc. When deployed, the application needs to request/receive these DICOM series, converts them into volumetric images, registers them, and then generates a multi-channel single image to feed the model
  1. Expected order and meaning of the channel, e.g. corresponding to T1, or T2 series
  • For flexibility in orchestrating the inference pipeline/Compose, there should not be implicit dependency in the inference config, e.g. MONAI Handlers depending on Ignite.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Feb 19, 2022

Another related PR relating to the configuration parsing: #3822.

Thanks.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Feb 21, 2022

Related PR for common training, evaluation and inference: #3832 .

Thanks.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Feb 21, 2022

As we are still developing the ConfigParser in PR #3822, I compared it with the Hydra OmegaConf: https://omegaconf.readthedocs.io/en/latest/index.html
There are several interesting features of it we still not support:

  1. Create or access config by dot list instead of dict keys:
    https://omegaconf.readthedocs.io/en/latest/usage.html#from-a-dot-list
  2. Create or override config by command line:
    https://omegaconf.readthedocs.io/en/latest/usage.html#from-command-line-arguments
    Added draft support in PR: 3482 add run API for common training, evaluation and inference #3832
  3. Support default value if the target node is not existing:
    https://omegaconf.readthedocs.io/en/latest/usage.html#default-values
  4. Use the value "???" to indicate mandatory values:
    https://omegaconf.readthedocs.io/en/latest/usage.html#mandatory-values
  5. Serialization: https://omegaconf.readthedocs.io/en/latest/usage.html#serialization
    We tried to support it with TorchScript saving.
  6. Relative interpolation for variables: https://omegaconf.readthedocs.io/en/latest/usage.html#variable-interpolation
    Similiar to our "@" mark with ID, but also support relative path.
  7. Merge configs: https://omegaconf.readthedocs.io/en/latest/usage.html#merging-configurations
    Draft support in PR: 3482 add run API for common training, evaluation and inference #3832
  8. Read-Only flag to protect config content: https://omegaconf.readthedocs.io/en/latest/usage.html#read-only-flag
  9. Debugger integration: https://omegaconf.readthedocs.io/en/latest/usage.html#debugger-integration

I think we can try to support 3 with enhanced idea: use the "@" mark to define a placeholder, then parser gives the value.
And we can try to support 4 to make the base config similar to "base class with abstractmethod" that must be overridden.

For other features we may consider later.

Thanks.

@ericspod
Copy link
Member

I have a PR now on the specification document #3834 which should have been a draft PR but oh well. It's very minimal compared to what @Nic-Ma prototyped here and lacks details about intent, use cases, design expectations, etc.

@wyli
Copy link
Contributor

wyli commented Feb 25, 2022

I think #3822 is now intuitive and flexible after a few rounds of refactoring. We may want to provide a system-wide flag to optionally disable eval() because it is too powerful and unsafe..

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Feb 28, 2022

I put all the links of related PRs to the task steps in the above ticket description.

Thanks.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 7, 2022

Related PR for inference example bundle: Project-MONAI/tutorials#604.

Thanks.

@wyli wyli changed the title Develop MVP of model package Develop MVP of model bundle Mar 11, 2022
@wyli
Copy link
Contributor

wyli commented Mar 20, 2022

follow-up from the dev meeting:

  • to generate a simple 'template' for monai bundle, including metadata and the required folder structure
  • explore further the hugging face API
  • look into integration with monai label
  • meeting/demo with monai deploy

@ericspod
Copy link
Member

ericspod commented Mar 21, 2022

Revising this tutorial to use our bundle would be a good exemplar: https://github.com/Project-MONAI/tutorials/blob/e3eea8704f5d002002f79cffec112c5c280476b4/modules/transfer_mmar.ipynb

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 22, 2022

After sharing with the internal NVIDIA Clara team, I put 2 missing features in the task feature.

 1. Config python logging properties in a file.
 2. Specify rank ID for component to run only on some ranks, for example, saving checkpoint in rank 0.

Thanks.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Mar 30, 2022

Feedback from MONAI deploy team:

  1. How to detect / avoid dead-lock in macro replacement.
  2. How to support real string starts with syntax, may add "" support.

Welcome more testing and feedback later.

Thanks.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Apr 4, 2022

Most of the tasks are completed, I created several new tickets to track the left items, and let's close this big first ticket now.

Thanks.

@Nic-Ma Nic-Ma closed this as completed Apr 4, 2022
Repository owner moved this from In Progress to Done in MONAI 0.9 Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants