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

[BUG]: download model-configurations is broken #6407

Closed
YohannParis opened this issue Feb 4, 2025 · 7 comments · Fixed by #6570
Closed

[BUG]: download model-configurations is broken #6407

YohannParis opened this issue Feb 4, 2025 · 7 comments · Fixed by #6570
Assignees
Labels
bug Something isn't working

Comments

@YohannParis
Copy link
Member

Describe the bug
downloading a model configuration should be updated to be named Download as configure model and call the following endpoint model-configurations/as-configured-model/

@YohannParis YohannParis added the bug Something isn't working label Feb 4, 2025
@dgauldie
Copy link
Collaborator

should be downloaded as a json

@dvince2
Copy link
Collaborator

dvince2 commented Feb 10, 2025

File seems to be downloading as designed. The .modelconfig file is actually a zip file containing both the model json and its config's json.

@dvince2
Copy link
Collaborator

dvince2 commented Feb 10, 2025

Methods to clean up:

ModelConfigurationController#downloadModelConfigurationWithModel - zip file with model and model config json
ModelConfigurationController#getConfiguredModel - Applies a configuration to a model, and returns the JSON for a new Model

@YohannParis
Copy link
Member Author

Then it should be a .zip file and not a .modelconfig file, user do not know what to do of this.

@dvince2
Copy link
Collaborator

dvince2 commented Feb 10, 2025

At the time we went with .modelconfig instead of zip because:

  1. We assumed that this was just for use in Terarium and not really to be consumed by a user
  2. By using .modelconfig we're able to filter on that extension in the file picker opened in the front end, so that users can only upload files we can consume (.pdf,.json,.csv,.modelconfig, etc). If we use .zip as an extension then our front end will have to be opened up to accept the more broad .zip extension.

modelconfigurations are terarium objects - are people trying to use that json for something external to terarium?

@YohannParis
Copy link
Member Author

@liunelson if you could weight on it.

In my opinion, we should download the model-configuratation as a configured model. So that the user can just re-upload it as is. I do not see a version where a user wants just a model-configuration JSON without its model.

@liunelson
Copy link
Member

Agreed. I understand a "configured model" as the model's AMR JSON with the selected configuration as its default (and only) configuration.

It should really be useful when a user wants to do some manual configuring and surgery in a text editor, where they can apply selective copy and replace.

However, one would still expect:

  • "Add to project" in the Model page sends the model with all its configs to the target project
  • "Add to project" in the Configure Model page sends the model with just this configuration

Such features are essentially in the case where one user works on the configuration and the other on the workflow (simulation, datasets, etc.).

dvince2 added a commit that referenced this issue Feb 11, 2025
Adding a new option to download a configured model in addition to the zipped archive
@dvince2 dvince2 linked a pull request Feb 11, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants