gyorilab / mira

MIRA modeling framework
BSD 2-Clause "Simplified" License
9 stars 7 forks source link

Epi Hackathon Scenario 5 stockflow.json does not pass validator #292

Open akruel01 opened 8 months ago

akruel01 commented 8 months ago
git clone https://github.com/gyorilab/mira.git
git clone https://github.com/DARPA-ASKEM/Model-Representations.git
cd Model-Representations/validation
python model_inventory.py --format md ../../mira/notebooks/hackathon_2024.02/scenario5/scenario_5_stockflow.json
source parameter distribution exists parameter dist/value set rate laws present rate law vars defined initial values present observables found
...rio5/scenario_5_stockflow.json False True missing rate semantics section missing rate semantics section missing state list 0
djinnome commented 8 months ago

The most recent version of scenario_5_stockflow.json still has remaining issues:

python ~/Projects/Proposals/ASKEM/Model-Representations/validation/model_inventory.py \ 
          --format md \
https://raw.githubusercontent.com/gyorilab/mira/main/notebooks/hackathon_2024.02/scenario5/scenario_5_stockflow.json
source parameter distribution exists parameter dist/value set rate laws present rate law vars defined initial values present observables found
...rio5/scenario_5_stockflow.json False True missing rate semantics section missing rate semantics section missing state list 0
bgyori commented 8 months ago

This analysis doesn't seem to be working correctly for the stock and flow AMR. This AMR represents flow rates directly on stocks. There is no "rate semantics section" like for the Petri net AMR. E.g.,

  "model": {
    "flows": [
      {
        "id": "1",
        "name": "new_cases_reported",
        "upstream_stock": null,
        "downstream_stock": "cumulative_cases_reported",
        "rate_expression": "disease_progression + isolation_rate_asym + isolation_rate_sym + q_disease_progress_rate",
        "rate_expression_mathml": "<apply><plus/><ci>disease_progression</ci><ci>isolation_rate_asym</ci><ci>isolation_rate_sym</ci><ci>q_disease_progress_rate</ci></apply>"
      },
...

I think there is likely also a problem with how the script above checks for "initial values present". These are available in the semantics/ode/initials section, e.g.,

        {
          "target": "susceptibles",
          "expression": "1339200000.0",
          "expression_mathml": "<cn>1339200000.0</cn>"
        },

@akruel01 and @djinnome, please check the validation script to see if it can be generalized to the stock and flow AMR. Still, I'm sure there could be other problems with the AMR that have to be sorted out. Please send these along, if you confirm they are indeed issues in the AMR.

JosephCottam commented 8 months ago

Conceptual note: The script is an inventory not a validator. It a tool to build a validator on, but I tried to just report what is/not present.

That being said, you are correct that it needs to be refined. I see two things:

  1. If a section is not expected in a case, the message should not say 'missing' but maybe something more like 'not present as expected' (and 'unexpectedly present' if present when not required).
  2. As you point out, it look for these value at this other location when in a stockflow.

@bgyori Does that sound fair?