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

Successive 2D constant arrays are merged incorrectly #7

Closed
ToddFincannon opened this issue Sep 14, 2021 · 2 comments · Fixed by #8
Closed

Successive 2D constant arrays are merged incorrectly #7

ToddFincannon opened this issue Sep 14, 2021 · 2 comments · Fixed by #8
Assignees
Labels
bug Something isn't working
Milestone

Comments

@ToddFincannon
Copy link
Collaborator

When a model contains two successive 2D constant arrays (after sorting into variable name order by the preprocessor) the equations are merged by the parser. For instance, this model:

DimA: A1, A2, A3 ~~|
DimB: B1, B2 ~~|

p[DimA, DimB]=
0, 0;
0, 0;
0, 0;
~~|

q[DimA, DimB]=
1, 1;
1, 1;
1, 1;
~~|

shows the following formula in the variable listing for the first equation.

= 0,0;0,0;0,0;q[DimA,DimB]=1,1;1,1;1,1;
@ToddFincannon ToddFincannon added the bug Something isn't working label Sep 14, 2021
@ToddFincannon ToddFincannon self-assigned this Sep 14, 2021
@ToddFincannon
Copy link
Collaborator Author

The only way to distinguish the end of the constant list in this case is to consider the ~~| sequence as an equation delimiter. We have not needed to do this in the past. We were able to rely on the next equation matching a parser rule instead.

It turns out we were already matching the ~~| sequence with the "unitsDoc" rule, just so we could discard it. To fix this problem, we add "unitsDoc" to the end of the equation and subscriptRange rules to explicitly match the equation delimiter. It needs to be optional, because SDEverywhere generates equations internally for things like levels in DELAY macro expansions without using the ~~| delimiter.

@ToddFincannon
Copy link
Collaborator Author

This parser change resulted in a large increase in code generation time. The cause is probably that unitsDoc is now a parser rule that injects large amounts of text into the parse tree instead of skipping it in the lexer.

We can handle this issue a different way by reverting to the UnitsDoc lexer rule, and simply terminating equations and subscript ranges on the | character. I did some tests and confirmed that the vertical bar character cannot be part of an equation or doc string. The UnitsDoc rule matches everything up to but not including the vertical bar.

This approach requires SDEverywhere to add the ~~| terminator to the end of equations that it generates for delay expansions, etc. We should probably have done this all along. See SDEverywhere issue #119 for a commit with the necessary changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants