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

Function to create an AMR model structure from model configuration #3811

Conversation

blanchco
Copy link
Contributor

@blanchco blanchco commented Jun 10, 2024

Description

  • create AMR from config

Resolves #(issue)

@blanchco blanchco changed the title 3759 add ta3 service endpoints for old model configurations schema Function to create an AMR model structure from model configuration Jun 10, 2024
Comment on lines 209 to 213
updateModelParameters(
model.get().getParameters(), modelConfiguration.get().getParameterSemanticList());
updateModelInitials(
model.get().getInitials(), modelConfiguration.get().getInitialSemanticList());
updateModelObservables(model.get().getObservables(), modelConfiguration.get().getObservableSemanticList());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using update instead of set here?
A model-configuration should match the model that is tied to the modelId so there is no reasons to do some matching of information here? Or I am missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ill rename the function to set instead of update.

This jist of these functions are to set the semantics in an AMR structure from the model it is derived from. The model semantics would have additional properties that we do not get from a model configuration (i.e. name, description, concept...). I am just simply setting the certain properties that we have in the model configuration to the model (value, distribution, expression...).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YohannParis fyi it isn't super clear that the config will match the model exactly, you may have extra observables and I don't think we have made it clear where observables should be defined. May be worth to clear up after the 1st around.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blanchco in that case you are right, what ever name you decide is good with me. No worries.
@mwdchang the Observable are set on the Model operator, and are port of the model object, no?

…' into 3759-add-ta3-service-endpoints-for-old-model-configurations-schema
@blanchco blanchco merged commit 6e7f1bf into spike/update-model-configuration Jun 11, 2024
@blanchco blanchco deleted the 3759-add-ta3-service-endpoints-for-old-model-configurations-schema branch June 11, 2024 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants