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

Bundle Specification Needs Required Packages #6959

Open
ericspod opened this issue Sep 7, 2023 · 7 comments · May be fixed by #7253
Open

Bundle Specification Needs Required Packages #6959

ericspod opened this issue Sep 7, 2023 · 7 comments · May be fixed by #7253
Assignees
Labels
Bundles Anything related to bundles enhancement New feature or request

Comments

@ericspod
Copy link
Member

ericspod commented Sep 7, 2023

Is your feature request related to a problem? Please describe.
Bundles can be specified to depend on optional packages through the optional_packages_version in metadata.json. Some bundles may require certain packages so a required_packages_version may be needed, or a change in name for optional_packages_version.

Describe the solution you'd like
Some way of specifying what packages other Pytorch, Numpy, and MONAI are needed by a bundle.

Describe alternatives you've considered
I've used optional_packages_version to specify needed packages but this doesn't seem adequate.

@ericspod ericspod added enhancement New feature or request Bundles Anything related to bundles labels Sep 7, 2023
@ericspod
Copy link
Member Author

ericspod commented Sep 7, 2023

I don't think this is a really high priority item, but we should update the metadata schema some time anyway.

@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 21, 2023

Hi @ericspod, for now, if optional_packages_version is in "metadata.json", it will add these packages to requirements.txt. Doesn't this currently meet the request, or do you think it would be better to change the name to required_packages_version?
https://github.com/Project-MONAI/model-zoo/blob/8845f3a04766e254b156c23fc4d2f91659c056d4/ci/get_bundle_requirements.py#L39

@ericspod
Copy link
Member Author

The issue is that there are required packages that a bundle absolutely needs and packages that would nice to have but not strictly needed. These two aren't differentiated in the json file and I felt there should be, or we change the name to required_packages_version to make it clear these packages are needed and not worry about having things optional.

@KumoLiu KumoLiu self-assigned this Nov 23, 2023
KumoLiu added a commit to KumoLiu/MONAI that referenced this issue Nov 23, 2023
Signed-off-by: KumoLiu <yunl@nvidia.com>
@KumoLiu KumoLiu linked a pull request Nov 23, 2023 that will close this issue
7 tasks
@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 23, 2023

Hi @Ericspo, I think we also need to create a new schema and update all bundles. What do you think?
https://github.com/Project-MONAI/model-zoo/blob/f20f894f2c0993c94184c7002529ea303a2f8c74/models/brats_mri_segmentation/configs/metadata.json#L2C16-L2C120

@ericspod
Copy link
Member Author

Yes we do need a new schema. I wanted to update it to try to take into account non-tensor arguments as well as arbitrary argument names. It currently isn't quite correct without so I need to do some looking into JSON schema definitions and get to that.

@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 24, 2023

What non-tensor arguments and arbitrary argument names did you refer to?
I also considering updating the output definition in the schema. For the raw output of the network, it is possible that there is more than just a single tensor, do you think we need to consider them all? Look like now we are defining the raw output of the network. But sometimes the output can be more complicated(such as detection).
#6724
https://github.com/Project-MONAI/model-zoo/blob/f20f894f2c0993c94184c7002529ea303a2f8c74/models/endoscopic_tool_segmentation/configs/metadata.json#L78

@ericspod
Copy link
Member Author

What non-tensor arguments and arbitrary argument names did you refer to?

Currently the schema allows only tensor arguments to a network's forward pass and we need to take into account other types. The initial design intent was to capture what tensor values are needed for input but to be a complete specification we need to account for other things. For outputs it might be the same thing where we need to account for more than just tensors as output, and different numbers of outputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bundles Anything related to bundles enhancement New feature or request
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

2 participants