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

Error required: check that units in AMR are internally consistent #216

Open
djinnome opened this issue Jul 10, 2023 · 9 comments
Open

Error required: check that units in AMR are internally consistent #216

djinnome opened this issue Jul 10, 2023 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@djinnome
Copy link
Contributor

djinnome commented Jul 10, 2023

We need unit tests that check to make sure that consistent dimensions and units are being used among model variables and parameters, and between the data and the model variables and parameters

@SamWitty
Copy link
Contributor

@djinnome , could you please complete the description for this issue or close it? Thanks!

@SamWitty SamWitty added the enhancement New feature or request label Dec 19, 2023
@SamWitty SamWitty changed the title Perform dimensional analysis consistency checks on AMR and outputs Error required: check that units in AMR are internally consistent Feb 14, 2024
@SamWitty
Copy link
Contributor

This should be able to handle the wastewater example, where different state variables have different units. To handle this, use the units on parameters and rate laws to reconcile differences.

@djinnome
Copy link
Contributor Author

djinnome commented Mar 22, 2024

I looked into the AMR, and there is no explicit annotation of dimensions or units. The closest thing is the DKG identifiers, and I don't think that has enough information to perform a dimensional analysis. I suppose we can assume that parameters are 1/day, that compartments are people and that fluxes are people/day, but without a set of clear use cases that would fail or pass, especially since the wastewater state variable has different units, I am not sure this is actionable.

@djinnome
Copy link
Contributor Author

I did find one model that had units: https://github.com/DARPA-ASKEM/simulation-integration/blob/main/data/models/SEIRHD_base_model01_petrinet.json

So at least I could perform a dimensional analysis on the rate law to make sure it works out to person/day.

@djinnome
Copy link
Contributor Author

Since we are already using sympy, I think https://docs.sympy.org/latest/modules/physics/units/index.html is probably the simplest way to perform dimensional analysis.

@djinnome
Copy link
Contributor Author

djinnome commented Mar 22, 2024

Questions:

@SamWitty
Copy link
Contributor

@djinnome , should this be re-opened now that you've found an AMR with units?

@djinnome
Copy link
Contributor Author

Re-opening, since a path to success exists.

@djinnome djinnome reopened this Mar 22, 2024
@djinnome
Copy link
Contributor Author

Honestly, there isn't much to do for Epi models, but I think the dimensional analysis will be critical for managing the complexity of climate models in phase 2.

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

When branches are created from issues, their pull requests are automatically linked.

2 participants