-
Notifications
You must be signed in to change notification settings - Fork 22
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
Conversation
There was a problem hiding this 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.
Fixes #87
Implement the QUANTUM function as a C macro with tests taken from the Vensim help documentation.