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

feat: implement DELAY FIXED function #108

Merged
merged 8 commits into from
Sep 27, 2021
Merged

Conversation

ToddFincannon
Copy link
Collaborator

@ToddFincannon ToddFincannon commented Sep 11, 2021

Fixes #29

DELAY FIXED requires something none of the other Vensim functions implemented so far need: auxiliary storage of past variable values. The number of values to be stored depends on the delay time, which is quantized to a multiple of TIME STEP. The storage buffer must be allocated at runtime, because the delay time is not known until then. To support this, a new FixedDelay structure is added that implements a ring buffer sized to the number of delay steps.

I followed the Vensim docs which mention that DELAY FIXED is a level var with a special fixed delay subtype. It behaves very much like a level, with a separate initial value set in the init phase, and with evaluation at the same time as other levels. The subtype indicates the special handling that is required, starting with the FixedDelay support structure mentioned above. Code is generated to allocate and initialize this structure at init time just after setting the initial value for the DELAY FIXED variable. Note that the FixedDelay structure is not a Vensim variable. It is generated as a special case.

The other special handling that is needed is: rewriting the DELAY FIXED equation at eval time to remove the initialization arguments (similar to INTEG) and add the FixedDelay support argument; and declaring the FixedDelay pointer at decl time.

When the DELAY FIXED variable is subscripted, the FixedDelay struct is also subscripted. It uses the LHS subscripts. Other variables occurring in arguments on the RHS are still free to use different subscripts.

A new _DELAY_FIXED Vensim function in C rounds out the implementation. It is coupled with __new_fixed_delay to allocate and initialize the FixedDelay structure. _DELAY_FIXED implements a straightforward ring buffer with one twist: when pulling a delayed value from the buffer, it increments the current position first. This is because DELAY FIXED is a level with a value evaluated for the next time step.

@ToddFincannon ToddFincannon added this to the 0.6.0 milestone Sep 11, 2021
@chrispcampbell chrispcampbell changed the title Todd/29 delay fixed feat: implement DELAY FIXED function Sep 22, 2021
Copy link
Contributor

@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.

@ToddFincannon Only spotted one potential issue that I noted in a comment. The rest looks great to me. Once you resolve that issue, this will be ready to merge.

src/c/vensim.c Outdated Show resolved Hide resolved
@ToddFincannon
Copy link
Collaborator Author

The suggested change worked after an additional fix. Because DELAY FIXED makes a level variable, when we check to see whether we have reached the delay time yet, we have to compare TIME to the following time step. This was true before, but the issue was masked by the fact that the initial value and early buffer values were both zero.

I added a new test model called delayfixed2 with a different INITIAL TIME and two different delay times.

Copy link
Contributor

@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.

The new fixes and tests look good. I'll merge this shortly.

@chrispcampbell chrispcampbell merged commit bd3b3e8 into develop Sep 27, 2021
@chrispcampbell chrispcampbell deleted the todd/29-delay-fixed branch September 27, 2021 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for DELAY FIXED
2 participants