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]: "Stratify" seems to make unexpected changes to model structure (overwrites a template) #411

Closed
liunelson opened this issue Dec 19, 2024 · 3 comments

Comments

@liunelson
Copy link

Before stratification:
Screenshot 2024-12-19 at 9 02 53 AM

After stratification:
Screenshot 2024-12-19 at 9 03 19 AM

This model was generated from LaTeX equations.
Model A (local, edited).json

Stratified model:
Model A (local, edited, strat).json

Note that the Sl(t) equation had a term with factor Sl(t) * v * (...) but it gets overwritten with Sh(t) * (...) after stratification:

model = stratify(
    template_model=model,
    key= "location",
    strata=['eu', 'af'],
    structure= [],
    directed=False,
    cartesian_control=False,
    modify_names=True,
    concepts_to_stratify=['Eh', 'P', 'I1', 'I2', 'H', 'Rh', 'Er', 'Ir', 'Rr', 'Sr', 'Sl', 'Sh'], #If none given, will stratify all concepts.
    concepts_to_preserve=None, #If none given, will stratify all concepts.
    params_to_stratify= None, #If none given, will stratify all parameters.
    params_to_preserve= None, #If none given, will stratify all parameters.
    param_renaming_uses_strata_names = True
)
@liunelson
Copy link
Author

@nanglo123 @kkaris @bgyori Could you take a look when convenient? This is a step in the Hackathon Decision Maker Scenario, where Model A is stratified into a Europe-Africa model.

@liunelson
Copy link
Author

I think Tenzin and I found the source of the problem: MIRA template_model_from_sympy_odes generates GroupedControlledConversion templates that do not have names so stratify creates bad stratified template names.

@nanglo123
Copy link
Contributor

So this issue actually stems from the fact that our stratification method will assign template names if no template names exist. If there are multiple template names under the same strata then they will have the same name. Since Terarium relies on template naming to assign rate laws to their appropriate template, this is where the issue of mismtached rate laws derives from.

It could be a discussion topic on whether we want to remove template renaming altogether and have Terarium rely on other parts of a template to identify them or if we should change our renaming process.

I'm closing the issue now as the issue has been partially adressed as we now make sure to add a template name for all template types during ingestion if they have a name, removing lossiness for template names in the end-to-end proccesing of models in MIRA. Maybe this topic could be repurposed as a feature request as I don't think it's necessarily a "bug" if we have "bad" template names as it may not be the best approach to identify templates just by name or to name templates if they don't have one.

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

No branches or pull requests

2 participants