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 QUANTUM function #89

Merged
merged 13 commits into from
Jul 26, 2021
Merged

feat: implement QUANTUM function #89

merged 13 commits into from
Jul 26, 2021

Conversation

ToddFincannon
Copy link
Collaborator

@ToddFincannon ToddFincannon commented Jul 23, 2021

Fixes #87

Implement the QUANTUM function as a C macro with tests taken from the Vensim help documentation.

@chrispcampbell chrispcampbell changed the title Todd/87 quantum feat: implement QUANTUM function Jul 24, 2021
@chrispcampbell chrispcampbell linked an issue Jul 24, 2021 that may be closed by this pull request
Base automatically changed from todd/81-direct-data-csv to develop July 26, 2021 22:15
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.

Looks good. I pushed a small fix to add parentheses around the macro arguments, which adds a layer of safety for when the macro args are expressions (similar to what we have for other macros).

BTW, I was on the fence about whether this should be a macro or a function; when I worked on reimplementing some things as macros vs functions last year, in cases where an arg appears multiple times on the right side, I sometimes chose to implement it as a C function to avoid redundant calculations. But in the case of QUANTUM, only b appears multiple times and (I'm assuming) it's likely to be a constant value rather than an expression, so I'm less worried about it. And either way, we're getting into micro-optimization territory, so I'm really not too worried about it.

@chrispcampbell chrispcampbell merged commit 42225f9 into develop Jul 26, 2021
@chrispcampbell chrispcampbell deleted the todd/87-quantum branch July 26, 2021 23:37
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.

Implement the QUANTUM function
2 participants