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

SUM loop is incorrect when multi-dimensional value accessed same dimension (marked and unmarked) #85

Closed
chrispcampbell opened this issue Jul 15, 2021 · 2 comments · Fixed by #588
Assignees

Comments

@chrispcampbell
Copy link
Contributor

I'm not sure if this is a typical use case, but in a recent version of the C-ROADS, the following equation was used and SDE emitted incorrect code, so I'm logging this issue while I remember the details.

Model equations that trigger the issue (this can be added to the existing sum.mdl test to reproduce):

DimT: T1, T2 ~~|
DimT': DimT ~~|
DimU: U1, U2, U3, U4 ~~|

t[DimT] = 1, 2 ~~|
u[DimU] = 10, 20, 30, 40 ~~|

t two dim[DimT,DimT'] = (10 * t[DimT]) + t[DimT'] ~~|
t two dim with u[DimT, DimT', DimU] = (10 * u[DimU]) + (10 * t[DimT]) + t[DimT'] ~~|
v[DimT] = SUM(t two dim[DimT, DimT!]) ~~|
w[DimT, DimU] = u[DimU] * SUM(t two dim[DimT, DimT!]) ~~|
x[DimT, DimU] = SUM(t two dim with u[DimT, DimT!, DimU]) ~~|

In short, we have some equations in C-ROADS that use a 2D array for expressing relationships between two dimensions, and in some places we have a SUM that involves both dimensions (for example, see v[DimT] above). SDE generates the following code for that one, which has the wrong indexes (it has _t_two_dim[v][v], which is incorrect; it should be _t_two_dim[i][v]:

  // v[DimT] = SUM(t two dim[DimT, DimT!])
  for (size_t i = 0; i < 2; i++) {
    double __t12 = 0.0;
    for (size_t v = 0; v < 2; v++) {
      __t12 += _t_two_dim[v][v];
    }
    _v[i] = __t12;
  }

As a workaround, we can use [DimT, DimT'!] ("DimT prime") instead of [DimT, DimT!] so that SDE treats them as two separate/distinct dimensions. The workaround is fine for now, but it would be safer if we could fix this issue in SDE.

@chrispcampbell
Copy link
Contributor Author

It appears that this issue is resolved by the fixes for #179. I will include the above sample model in that branch (and will update the unit tests to cover these cases) and will mark this issue as being "fixed" by the forthcoming PR.

@chrispcampbell
Copy link
Contributor Author

For reference, here is the generated code for v[DimT] after the fix (compare to the code in the description above).

  // v[DimT] = SUM(t two dim[DimT,DimT!])
  for (size_t i = 0; i < 2; i++) {
  double __t17 = 0.0;
  for (size_t u = 0; u < 2; u++) {
    __t17 += _t_two_dim[i][u];
  }
  _v[i] = __t17;
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant