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

[FEATURE] Support register_model with a dict parameter model_config #390

Open
yuye-aws opened this issue May 21, 2024 · 4 comments
Open
Labels
enhancement New feature or request

Comments

@yuye-aws
Copy link
Member

Is your feature request related to a problem?
The register_model method in ml_commons_client.py only accepts model_config_path as the input for model_config.

What solution would you like?
A clear and concise description of what you want to happen.

The register_model method can also take in a dict parameter such as model_config

What alternatives have you considered?
A clear and concise description of any alternative solutions or features you've considered.

Do you have any additional context?
Add any other context or screenshots about the feature request here.

@yuye-aws yuye-aws added enhancement New feature or request untriaged labels May 21, 2024
@Arnav-Gr0ver
Copy link

Arnav-Gr0ver commented Jun 6, 2024

Hello @yuye-aws, wanted to clarify something on this feature.

I checked the register model method and when the model is uploaded there is a check against the mandatory fields - a dict parameter such as model_config doesn't contain all the necessary parameters i.e. name, version, format in comparison to current implementation of a json file with all the metadata.

So, are you suggesting there should be an alternate parameter for a dict which contains all the mandatory parameters along with the model config dict (guessing this would be a nested dict)?

@dblock
Copy link
Member

dblock commented Jun 24, 2024

Catch All Triage - 1 2 3 4 5 6

@dblock dblock removed the untriaged label Jun 24, 2024
@dhrubo-os
Copy link
Collaborator

@yuye-aws could you please clarify your question further?

@yuye-aws
Copy link
Member Author

So, are you suggesting there should be an alternate parameter for a dict which contains all the mandatory parameters along with the model config dict (guessing this would be a nested dict)?

Exactly yes. When calling register_model method, not every user wishes to create a json file as the model config. It would be better if this method accepts a new dict parameter like model_config.

@yuye-aws yuye-aws changed the title [FEATURE] Support register_model with a json file [FEATURE] Support register_model with a dict parameter model_config Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants