One model of PNLib got broken

[casella]

The PNLib library by @lochel worked 100% in omc 1.18.0, see overview report. For some reason, something got wrong in the current 1.19.0-dev version, causing the ArcswithPCtoTC3 model to fail at runtime with this error:

stdout            | info    | Using sparse solver kinsol for nonlinear system 1 (1435),
|                 | |       | because density of 0.05 remains under threshold of 0.10.
stdout            | info    | The maximum density for using sparse solvers can be specified
|                 | |       | using the runtime flag '<-nlsMaxDensity=value>'.
assert            | debug   | Solving non-linear system 1435 failed at time=0.
|                 | |       | For more information please use -lv LOG_NLS.
assert            | info    | simulation terminated by an assertion at initialization

This is kind of weird, I don't understand why nonlinear systems should be involved in the solution of PN models at all. @lochel, could you comment on that?

@mahge, @adrpo, could you help me finding when the regression took place? I remember you showed me how to do that, but I lost track of it.

If nonlinear systems are not to be expected, then I guess @kabdelhak should check if there is something wrong with causalization.

[mahge]

This is the regression history of the model on master from latest to oldest

1. http://libraries.openmodelica.org/branches/history/master/2021-11-16%2011:41:32..2021-11-16%2022:23:49.html
2. http://libraries.openmodelica.org/branches/history/master/2021-11-12%2008:56:38..2021-11-12%2022:41:02.html
3. http://libraries.openmodelica.org/branches/history/master/2021-04-15%2023:00:29..2021-04-16%2012:12:09.html
4. http://libraries.openmodelica.org/branches/history/master/2021-04-14%2022:57:15..2021-04-15%2023:00:29.html
5. http://libraries.openmodelica.org/branches/history/master/2021-03-23%2023:56:51..2021-03-24%2011:47:25.html
6. http://libraries.openmodelica.org/branches/history/master/2020-12-17%2022:59:27..2020-12-18%2023:05:17.html
7. http://libraries.openmodelica.org/branches/history/master/2020-12-15%2023:23:53..2020-12-17%2000:28:56.html
8. http://libraries.openmodelica.org/branches/history/master/2020-10-16%2018:11:51..2020-10-22%2015:54:24.html

It went from Verfiy to FrontEnd (failing in the backend) at 2 and then went from FrontEnd to Compile in 1.

Since it went Verify -> FrontEnd -> Compile between 2021-11-12 and 2021-11-16, its simulation behavior might be affected by anything done between those dates as well.

[casella]

Thanks @mahge!

@AnHeuermann you were mostly involved with those commits, could you please check what went possibly wrong with ArcswithPCtoTC3 in those four days? Thanks!

[AnHeuermann]

I don't think the error should be because of 1cbaefaede. In that case the error message should be different. But I can try to quickly build and test on that an the previous commit.

[AnHeuermann]

I checked the PNlib.Examples.ExtTest.ArcswithPCtoTC3 with different commits:

• 3602522: Simulates, but I get an error from the backend:

  "Warning: The Tearing heuristic has chosen variables with annotation attribute 'tearingSelect = avoid'. Use -d=tearingdump and -d=tearingdumpV for more information.
  [/home/andreas/workspace/OpenModelica/OMCompiler/Compiler/BackEnd/Differentiate.mo:265:5-265:157:writable] Error: Derivative of expression \"$res_NLSJac0_6 := min(min(min({P1.decFactorOut[2], P3.decFactorOut[2], P5.decFactorOut[2], P7.decFactorOut[2]}), min({})), P7.prelimSpeedOut[2]) - T5.instantaneousSpeed\" w.r.t. \"dummyVarNLSJac0\" is non-existent.
  Error: Internal error SymbolicJacobian.deriveAll failed
  [/home/andreas/workspace/OpenModelica/OMCompiler/Compiler/BackEnd/SymbolicJacobian.mo:2261:7-2261:87:writable] Error: Internal error function generateSymbolicJacobian failed
  [/home/andreas/workspace/OpenModelica/OMCompiler/Compiler/BackEnd/SymbolicJacobian.mo:2053:9-2053:79:writable] Error: Internal error function createJacobian failed
  Warning: The initial conditions are not fully specified. For more information set -d=initialization. In OMEdit Tools->Options->Simulation->Show additional information from the initialization process, in OMNotebook call setCommandLineOptions(\"-d=initialization\").
  Warning: The Tearing heuristic has chosen variables with annotation attribute 'tearingSelect = avoid'. Use -d=tearingdump and -d=tearingdumpV for more information.
  "
  

• 2e9ac99 and 3dba97e: Same as above
• 2baf5e9: Frontend (Backend?) errored:

  "Error: An independent subset of the model has imbalanced number of equations (467) and variables (463).
  [...]
  Error: pre-optimization module encapsulateWhenConditions (simulation) failed.
  

• 38c9f79: Still broken

Maybe it could be 8df2386 if PNLib uses mod(), but hard to tell without removing single commits after the Frontend issue was solved again.

[AnHeuermann]

But bbbca3c is actually mentioning PNLib in its commit message. Sounds like a hint.
@perost could you check?

[perost]

I broke a lot of PNlib in #8117, which was intended to work around issues where references to 0-size arrays remained in the flat model. Since it didn't work with PNlib for unknown reasons I reverted it in #8140 and instead attempted to fix the issue by replacing such crefs with appropriate array expressions, so I assume that's what broke ArcswithPCtoTC3.

Reverting everything is not an option since it fixes many issues in Buildings and other libraries, and it also fails in a similar way when using the old frontend. So it would be good if someone familiar with the backend could take a closer look, it might be something we need to change in the frontend but I'd need some guidance on what exactly is causing the issue.

[kabdelhak]

i mistakenly linked this issue to a PR which solves another, sorry!

Although the changes might actually fix this, let's see!

[casella]

@kabdelhak, last ditch reminder, could you have a look before 1.19.0 is released. @perost gave some hints in his comment.

[kabdelhak]

@casella Looking into it this week.

[kabdelhak]

I looked at PNlib.Examples.ExtTest.ArcswithPCtoTC3 and realized that after frontend there is the equation

  T5.instantaneousSpeed = min(min(min(T5.decreasingFactorIn), min({})) * T5.maximumSpeed, T5.prelimSpeed);

Which mathematically makes no sense due to min({}). The minimum element of an empty set is not defined. I looked into the specification and it does not say anything about that as far as i can see. I think we should issue an error in the frontend though.

I adapted the PNLib (PNlib.Components.TC) to properly model this:
WRONG:

instantaneousSpeed=min(min(min(decreasingFactorIn), min(decreasingFactorOut))*maximumSpeed, prelimSpeed);

CORRECT:

  if nIn == 0 and nOut == 0 then
    instantaneousSpeed = prelimSpeed;
  elseif nIn == 0 then
    instantaneousSpeed=min(min(decreasingFactorOut)*maximumSpeed, prelimSpeed);
  elseif nOut == 0 then
    instantaneousSpeed=min(min(decreasingFactorIn)*maximumSpeed, prelimSpeed);
  else
    instantaneousSpeed=min(min(min(decreasingFactorIn), min(decreasingFactorOut))*maximumSpeed, prelimSpeed);
  end if;

We could also work with some kinds of infinity tokens, but as far as i can see there is no syntax for that in modelica so it is not allowed?

Furthermore i have to say that this just fixes part of the regression and the model passes to simulation again. It still fails to initialize though.

[kabdelhak]

Using a dense solver instead of a sparse solver works for the initialization (simflags="-nlssMaxDensity=0.05" suffices).

I don't really know much about this kinsol sparse solver and why it fails here...

[sjoelund]

Which mathematically makes no sense due to min({}). The minimum element of an empty set is not defined. I looked into the specification and it does not say anything about that as far as i can see. I think we should issue an error in the frontend though.

https://specification.modelica.org/master/arrays.html#reduction-expressions says the result is defined as the "Greatest value of type".

[perost]

Which mathematically makes no sense due to min({}). The minimum element of an empty set is not defined. I looked into the specification and it does not say anything about that as far as i can see. I think we should issue an error in the frontend though.

The specification is a bit weird in this regard, because it does define min(... for e in {}) to be the largest value of the type (10.3.4.1). However, it does not define that min(A) is equivalent to min(A[j, k, …] for j, k, …) like it does for sum and product, so the value of min({}) is technically undefined. But for the sake of consistency I think we should consider that to be the case anyway.

[kabdelhak]

So should we do it in the Frontend? Since when you evaluate the array to be empty the type also vanishes right? We would need to do it at that point to know what the return type is.

[perost]

So should we do it in the Frontend? Since when you evaluate the array to be empty the type also vanishes right? We would need to do it at that point to know what the return type is.

Yes, I will add a simplification rule or something for it.

[perost]

I changed the NF to evaluate min/max of empty arrays in #8904. I think we avoided doing that on purpose earlier because that's what the OF did, so I'm not entirely sure if this is a good idea. But I guess we'll see.

[kabdelhak]

I changed the NF to evaluate min/max of empty arrays in #8904. I think we avoided doing that on purpose earlier because that's what the OF did, so I'm not entirely sure if this is a good idea. But I guess we'll see.

Thank you! For now i cannot see any disadvantage of evaluating this but time will tell i guess.

We already found the problem for the remaining issue of the sparse solver failing. Apparently functions cannot be partially differentiated and therefore any jacobian containing a function is computed numerically (i did not know that, in the NB it aready should be possible), That would not be much of an issue since in the case there is a function which would force the Backend to do it numerically, but somehow it misses this fact due to the tuple assignment.

  (a,b) = f(...);

This also gets detected as function call, but only if b is not of size 0. If it is of size 0 the backend somehow misses this fact and just fails on differentiation in result creating a jacobian that is missing an equation. Which of course is singular.

The quick solution would be to catch this earlier and just compute the numerical one. I am currently trying to find where that happens. The most correct way would be to implement partial function differentiation properly, but that takes much time and i don't know if its worth since it will work in the NB.

[perost]

@kabdelhak: Is the backend generating such equations? The frontend should remove 0-sized variables that are assigned in tuples, since those variables don't exist in the flat model. So if you get such equations from the frontend then there's a bug somewhere.

[kabdelhak]

@kabdelhak: Is the backend generating such equations? The frontend should remove 0-sized variables that are assigned in tuples, since those variables don't exist in the flat model. So if you get such equations from the frontend then there's a bug somewhere.

Nono it gets removed, i guess the underscore is the real problem:

  (a, _) = f(...);

By the way, are you sure your evaluation thing for empty arrays works? It did not work for the PNLib model and i tried this minimal model:

model min_test
  Real a;
  Real[N] b = fill(0,N);
  parameter Integer N = 0;
equation
  a = min(min(b), 1);
end min_test;

It did not seem to work, maybe i am missing something? Or maybe it does not work for nested calls?

[perost]

By the way, are you sure your evaluation thing for empty arrays works? It did not work for the PNLib model and i tried this minimal model:

model min_test
  Real a;
  Real[N] b = fill(0,N);
  parameter Integer N = 0;
equation
  a = min(min(b), 1);
end min_test;

It did not seem to work, maybe i am missing something? Or maybe it does not work for nested calls?

Ah, sorry. I didn't consider that we replace 0-sized variables with {} quite late in the frontend, so my fix doesn't work in that case. I'll add a proper simplification rule for it, and maybe I'll even consider checking that it actually works with the PNlib model too 😐

[perost]

The simplification of min({}) and max({}) should be fixed properly in #8909.

[casella]

@perost, @kabdelhak, thanks for the good work!

Once we see proof that the problem is resolved on master, please cherry-pick it to maintenance/v1.19, so we avoid the regression in the upcoming release.

[kabdelhak]

PR #8913 fixes the problem. I expanded the module evalFunctions to also remove these wildcard outputs. As a result they can be properly reformed to residuals and are not omitted from the loop equations. The jacobian module detects the function calls and decides to compute it numerically.

[kabdelhak]

Should i take the commit from @perost and mine and cherry-pick that to the v1.19? Is there a third one that needs to be included?
Or should we wait until library testing works to see the actual impact?

[perost]

Should i take the commit from @perost and mine and cherry-pick that to the v1.19? Is there a third one that needs to be included? Or should we wait until library testing works to see the actual impacts?

If you cherry-pick you should take both of my commits, i.e. #8904 and #8909. But yeah, we should probably wait for the library tests first.

[casella]

If you cherry-pick you should take both of my commits, i.e. #8904 and #8909. But yeah, we should probably wait for the library tests first.

Yes, I definitely suggest that.

[casella]

Just tried the ArcswithPCtoTC3 model on the latest 1.20.0-dev nightly build on my PC and it works fine.

@kabdelhak if you can take care of the cherry-picking into maintenance, I can then test it with the 1.19.0-beta.2, and then close the ticket if all's fine there. Thank!

[casella]

PNLib is now back at 100% also on the CI test results. All's well that ends well 😃

@kabdelhak can you please cherry-pick your fixes into maintenance/v1.19 ASAP? We need to go ahead with the release. Thanks!

[kabdelhak]

Cherry-Pick PR: #8974

[casella]

Works fine in 1.19.0-dev.beta4