Skip to content

feat: add initial support for XMILE/Stella models#778

Open
chrispcampbell wants to merge 82 commits intomainfrom
chris/468-xmile
Open

feat: add initial support for XMILE/Stella models#778
chrispcampbell wants to merge 82 commits intomainfrom
chris/468-xmile

Conversation

@chrispcampbell
Copy link
Contributor

Fixes #468

@ToddFincannonEI: Sorry for the large PR. I could've broken it up into 3 PRs but figured it would be better for you to see how it all fits together. I recommend first reading the issue comment that I just posted for context. I will annotate files and code in the PR to help provide more context.

The main thing is that none of this should have an impact on your/our existing models.

I'm cc'ing @bobeberlein mainly as a heads up. Bob, you're welcome to review parts and provide feedback if you'd like, but it's not critical since I know the code base is unfamiliar, and you're also welcome to ignore this completely 😄

Map XMILE/Stella's SAFEDIV function to the equivalent Vensim ZIDZ function.
SAFEDIV performs safe division, returning 0 when the denominator is zero.

Generated with Claude Code
Map XMILE/Stella's DELAY function to the equivalent Vensim DELAY FIXED function.
DELAY is a fixed-delay function that outputs a past value of the input after
a specified delay time.

Generated with Claude Code
Map XMILE/Stella's SIZE function to the equivalent Vensim ELMCOUNT function.
SIZE returns the number of elements in a dimension at compile time.

Generated with Claude Code
…E models

- Add _DEPRECIATE_STRAIGHTLINE case to validateStellaFunctionCall()
- Add _ACTIVE_INITIAL case to validateStellaFunctionCall()
- Update parse-xmile-variable-def.ts to synthesize ACTIVE_INITIAL calls
- Add tests for both functions
- Add PLAN.md documenting XMILE integration test analysis

7 of 11 XMILE integration tests now pass.

Generated with Claude Code
Copy link
Contributor Author

@chrispcampbell chrispcampbell left a comment

Choose a reason for hiding this comment

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

I added some comments to help add context for review.

choices: ['js', 'c'],
default: 'js'
},
tooldat: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this option for sde test so that we can pass a custom name for the dat file. Before this, it assumed that the file is called {model}.dat.

I could've also updated sde test to support reading csv files in addition to dat, but I didn't want to go deep on something that's probably not used much outside of our own testing framework. So instead I just added some code to our test scripts to convert the csv files (for expected Stella results) to dat format as part of the testing process.


/**
* Normalize a model pathname that may or may not include the .mdl extension.
* Normalize a model pathname that may or may not include the .mdl or .xmile/.stmx/.itmx extension.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The sde CLI commands previously automatically found the corresponding mdl file for a given model name. Now we do the same thing for xmile/stmx/itmx files. If there are multiple options, it throws an error and the user can specify which one by providing a full file name with extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The existing code gen tests were written using Vensim format as the input model format, but we need new tests that cover similar things except for XMILE format (since the code gen can be different in certain cases), so I renamed these files accordingly.

//
//

// Simple functions that are common to Vensim and XMILE/Stella
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this big switch statement so that it's more clear which cases are for both Vensim+XMILE, and which cases are for Vensim only or XMILE/Stella only.

And I added comments to note any format-specific things.

}

describe('readEquations', () => {
describe('readEquations (from Vensim model)', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I renamed this file to make it focused on reading equations in Vensim format. There is a new copy of these tests focused on reading equations in XMILE format.

I added one new test here to align with a test that I added in the XMILE tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the create package so that it handles stmx files in the same way that we already handled mdl files.


/** A complete model definition, including all defined dimensions and equations. */
export interface Model {
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the only addition I made at the AST level. We needed a way to include the simulation parameters because (unlike Vensim) they're in a different part of the XMILE structure than the equations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the record, I wrote nearly all of the test code in this PR myself, but really wish Claude was around when I was first working on this since the tests are very tedious to write. (I started this long before AI tools were a daily use kind of thing.) We could get even better test coverage by letting Claude have at it, but I'll save that for another day.


if [[ $INPUT_FORMAT == "stmx" ]]; then
# Skip tests for models that are not yet supported by the Stella/XMILE backend
if [[ $m == "allocate" || $m == "arrays" ]]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As noted in my issue comment, we are skipping these two tests because they are failing (only in part). I filed separate issues to address these separately.

"ci:build": "run-s clean lint prettier:check type-check test:ci build docs"
},
"dependencies": {
"@rgrove/parse-xml": "^4.1.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have one new dependency in the parse package, which is not used in the case of Vensim models. The good news is that the parse-xml package is pretty minimal and has no dependencies of its own.

@travisfranck
Copy link
Collaborator

I'm excited by this PR! Likely using Stella for our next project, so hopefully we can use SDE too! Thank you in advance, Chris.

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.

XMILE input support

2 participants