Specifying -s dassl fails simulation, but not specifying it (and using hence dassl as default) works

[mwetter]

Description

In our CI tests, we run command such as

./IBPSA.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.Validation.Measured_SmallScale -s dassl  -cpu -lv LOG_STATS

This model fails (IBPSA master, 75a5c6909cba535b96e75f60694dcded844b9e55) and OMC, 1.21.0_dev-14-gdf98762-1.

However, if run as

./IBPSA.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.Validation.Measured_SmallScale -cpu -lv LOG_STATS

it simulates. The screen output below shows that if dassl is not specified, omc uses dassl as default. Hence, I would expect specifying -s dassl should not have any side effect.

Steps to Reproduce

Translate model with

$ cat IBPSA.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.Validation.Measured_SmallScale_translate.mos
setCommandLineOptions("-d=nogen");
setCommandLineOptions("-d=initialization");
setCommandLineOptions("-d=backenddaeinfo");
setCommandLineOptions("-d=discreteinfo");
setCommandLineOptions("-d=stateselection");
setMatchingAlgorithm("PFPlusExt");
setIndexReductionMethod("dynamicStateSelection");
loadFile("IBPSA/package.mo");
translated := translateModel(IBPSA.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.Validation.Measured_SmallScale, method="dassl", tolerance=1e-6, numberOfIntervals=500, variableFilter="(meaDat.y[[]4[]]|TBorHol.y|add.y)");
getErrorString();

if translated then
  retVal := system("make -f IBPSA.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.Validation.Measured_SmallScale.makefile");
else
  print("Translation failed.");
  retVal := 10;
end if;
exit(retVal);

and simulate as above.

Expected Behavior

Specifying -s dassl should not have any side effect.

Screenshots

mwetter@srg-mw:/tmp/tmp-IBPSA-0-713i83i3$ ./IBPSA.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.Validation.Measured_SmallScale  -cpu -lv LOG_STATS
stdout            | info    | ... loading "data" from "/tmp/tmp-IBPSA-0-713i83i3/IBPSA/Resources/Data/Fluid/Geothermal/Borefields/HeatTransfer/Validation/Cimmino_Bernier_2015_SmallScale.txt"
stdout            | info    | ... loading "TStep" from "tmp/temperatureResponseMatrix/e4cb749a2d23f6e3b3b081cd674dae01371ab0c4TStep.mat"
LOG_SUCCESS       | info    | The initialization finished successfully without homotopy method.
LOG_STATS         | info    | ### STATISTICS ###
|                 | |       | | timer
|                 | |       | | |    0.0134426s          reading init.xml
|                 | |       | | |   0.00091831s          reading info.xml
|                 | |       | | |  0.000424191s [  0.0%] pre-initialization
|                 | |       | | |      1.80223s [ 14.9%] initialization
|                 | |       | | |   3.0978e-05s [  0.0%] steps
|                 | |       | | |      1.48478s [ 12.3%] solver (excl. callbacks)
|                 | |       | | |     0.153824s [  1.3%] creating output-file
|                 | |       | | |      4.98618s [ 41.2%] event-handling
|                 | |       | | |     0.558285s [  4.6%] overhead
|                 | |       | | |      3.10755s [ 25.7%] simulation
|                 | |       | | |      12.0933s [100.0%] total
|                 | |       | | events
|                 | |       | | | 10079 state events
|                 | |       | | | 236251 time events
|                 | |       | | solver: dassl
|                 | |       | | | 2451745 steps taken
|                 | |       | | | 2462057 calls of functionODE
|                 | |       | | | 2206216 evaluations of jacobian
|                 | |       | | |   935 error test failures
|                 | |       | | |     0 convergence test failures
|                 | |       | | | 1.45637s time of jacobian evaluation
LOG_SUCCESS       | info    | The simulation finished successfully.
mwetter@srg-mw:/tmp/tmp-IBPSA-0-713i83i3$ ./IBPSA.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.Validation.Measured_SmallScale -s dassl  -cpu -lv LOG_STATS
stdout            | info    | ... loading "data" from "/tmp/tmp-IBPSA-0-713i83i3/IBPSA/Resources/Data/Fluid/Geothermal/Borefields/HeatTransfer/Validation/Cimmino_Bernier_2015_SmallScale.txt"
assert            | error   | Solving a linear system of equations with function
|                 | |       | "Matrices.solve" is not possible, because the system has either
|                 | |       | no or infinitely many solutions (A is singular).
assert            | info    | simulation terminated by an assertion at initialization

Version and OS

OpenModelica 1.21.0~dev-14-gdf98762

Linux Ubuntu 20.04

Additional Context

This happens in our CI tests with BuildingsPy. Before changing BuildingsPy to only specify the solver if it is not dassl, it would be good to see if this should rather be corrected in OpenModelica.

[casella]

@AnHeuermann, @mahge any clue?

[mahge]

Nop. It's baffling. I will take a look.

[mahge]

The variable Done declared here in function IBPSA.Fluid.Geothermal.Borefields.BaseClasses.HeatTransfer.ThermalResponseFactors.gFunction is being used uninitialized in deciding how the thermal response matrix is initialized. This leads to the matrix A not being initialized properly. If Data is initialized properly, e.g,

Done[nBor, nBor] = fill(0,nBor,nBor)

everything works as expected.

The fact that specifying -s dassl causes this to happen is just pure coincidence as far as I can tell. I guess the exact location where the uninitialized Data points to changes slightly depending on what we allocate/not-allocate for additionally specified flags. It just happened to affect Data right now. I guess there is not much arguing with Undefined Behavior. This is as undefined behavior'ish as I have seen.

What is fascinating to me is that I could reproduce what happened to you exactly and repeatedly without fail. If not, this would have been even more problematic. I guess the gods smiled on me today.

Anyway, the proper solution to avoid these kinds of problems is to warn users on usage of uninitialized variables in functions. The old Front-end used to do that and @perost had plans to implement this for the new one IRRC.

[mwetter]

@mahge : Thanks for catching this. I will correct it in the IBPSA and Buildings libraries. That is indeed an unexpected side effect...

[mahge]

You are welcome.

[casella]

Bravo to @mahge for figuring this out. Side effects an unspecified behaviour are indeed a thing in CS 😄.

I just opened #9717 so that @perost's plan is officially made public and recorded.