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

OM fails to compile model with boolean nominals #6977

Closed
filip-jezek opened this issue Nov 24, 2020 · 13 comments
Closed

OM fails to compile model with boolean nominals #6977

filip-jezek opened this issue Nov 24, 2020 · 13 comments
Assignees

Comments

@filip-jezek
Copy link

Description

OpenModelica has problems generating equation for complex boolean variables. Here I demonstrate it on a simple circulatory. case

When the ideal diode is extended by chattering protection, then it fails to compile. Remove the chattering protection and it runs fine.

The compilation produces translation error for each of the four valves in the messages browser:

[8] 12:12:21 Translation Error
Internal error - BackendDAEOptimize.makeEquationToResidualExp failed to transform equation: TricuspidValve.open = TricuspidValve.passableVariable > 0.0 to residual form!

and in the simulation ouptut window:

Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm_02nls.c: In function 'initializeStaticDataNLS279':
Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm_02nls.c:89:97: error: 'BOOLEAN_ATTRIBUTE {aka struct BOOLEAN_ATTRIBUTE}' has no member named 'nominal'
sysData->nominal[i] = data->modelData->booleanVarsData[15].attribute /* TricuspidValve.open /.nominal;
^
Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm_02nls.c:90:97: error: 'BOOLEAN_ATTRIBUTE {aka struct BOOLEAN_ATTRIBUTE}' has no member named 'min'
sysData->min[i] = data->modelData->booleanVarsData[15].attribute /
TricuspidValve.open /.min;
^
Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm_02nls.c:91:97: error: 'BOOLEAN_ATTRIBUTE {aka struct BOOLEAN_ATTRIBUTE}' has no member named 'max'
sysData->max[i++] = data->modelData->booleanVarsData[15].attribute /
TricuspidValve.open */.max;

Probably tied to this issue https://trac.openmodelica.org/OpenModelica/ticket/5783

Steps to reproduce

Grab https://github.com/filip-jezek/Physiolibrary/tree/NoSteadyState and simulate Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm

Expected behavior

Should run. Doesnt run.

Version and OS

  • OpenModelica Version: 1.16.1
  • OS: Windows 10, 64 bit

any workarounds appreciated.

@mahge
Copy link
Contributor

mahge commented Nov 24, 2020

Yes this is the same issue as the one you linked (https://trac.openmodelica.org/OpenModelica/ticket/5783)

You can try compiling your models with the flag --tearingMethod=minimalTearing added. That might solve it for now until we put a proper fix for it.

@filip-jezek
Copy link
Author

@mahge Thanks, that seems to do the trick with this issue! However some other problems emerged.

I let to decide yourself, if to close this, or keep it open to fix. The proposed workaround works, although maybe because of that its impossible to finish the simulation.

@AnHeuermann
Copy link
Member

@filip-jezek If I understand you correctly you did not disable tearing?
Then this would be a problem in the tearing method used.

But this is the exact type of error for which minimalTearing was implemented. Whenever someone (or omc itself) disables tearing we have this kind of problems with algebraic loops that contain non-real variables.
We want to have minimalTearing to replace no tearing completely, but at the moment the method is not robust enough.


Btw. Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm simulates just fine for me:

loadFile("Physiolibrary/package.mo"); getErrorString();
simulate(Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm); getErrorString();

using OMCompiler v1.17.0-dev.195+g637d5adedc on Ubuntu 20.04 as well as OpenModelica v1.17.0-dev-198-gebd255f7fb on Windows 10.
@filip-jezek Could you test again with a nightly build?

$ omc runTest.mos
true
"Notification: Automatically loaded package Modelica 3.2.3 due to uses annotation.
Notification: Automatically loaded package Complex 3.2.3 due to uses annotation.
Notification: Automatically loaded package ModelicaServices 3.2.3 due to uses annotation.
"
record SimulationResult
    resultFile = "/home/andreas/workspace/Testitesttest/Physiolibrary/Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm_res.mat",
    simulationOptions = "startTime = 0.0, stopTime = 5.0, numberOfIntervals = 500, tolerance = 1e-06, method = 'dassl', fileNamePrefix = 'Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm', options = '', outputFormat = 'mat', variableFilter = '.*', cflags = '', simflags = ''",
    messages = "LOG_SUCCESS       | info    | The initialization finished successfully without homotopy method.
LOG_SUCCESS       | info    | The simulation finished successfully.
",
    timeFrontend = 1.2736808,
    timeBackend = 0.06431870000000001,
    timeSimCode = 0.0106874,
    timeTemplates = 0.04923590000000001,
    timeCompile = 0.9961215,
    timeSimulation = 0.09632149999999995,
    timeTotal = 2.4907442
end SimulationResult;
"Notification: Physiolibrary requested package Modelica of version 3.2.2. Modelica 3.2.3 is used instead which states that it is fully compatible without conversion script needed.
Warning: The initial conditions are not fully specified. For more information set -d=initialization. In OMEdit Tools->Options->Simulation->OMCFlags, in OMNotebook call setCommandLineOptions(\"-d=initialization\").
"

@filip-jezek
Copy link
Author

@AnHeuermann Originally I have not disabled tearing, once I use the --tearingMethod=minimalTearing it runs correctly.

Have you used the Physiolibrary version I have linked? It has some new modifications (that is chattering protection) which are causing the problem and which are not present in the Physiolibrary included in the OpenModelica.

Once you confirm you used the updated Physiolibrary, I can definitely test the nightly build.

@filip-jezek
Copy link
Author

I have further problems with more complex model, but I was unable to isolate or name it, so I created a forum post instead of a bug report here https://openmodelica.org/forum/default-topic/3148-om-fails-to-run-with-compilation-errors,-dymola-runs-fine#p10372

@AnHeuermann
Copy link
Member

AnHeuermann commented Nov 25, 2020

Have you used the Physiolibrary version I have linked?

My bad, now I'm getting the error as well.

It's an error in Cellier Tearing we need to fix. But the example is a bit to big to debug it.
I tried to come up with a more minimal example but did not succeed yet.

@mahge
Copy link
Contributor

mahge commented Nov 25, 2020

We should probably close one of the two tickets to keep the discussion in one place. @AnHeuermann which one do you want to keep?

@AnHeuermann
Copy link
Member

I prefer the GitHub issues, so I closed https://trac.openmodelica.org/OpenModelica/ticket/5783.

@kabdelhak
Copy link
Contributor

@AnHeuermann and I looked at this ticket and have a pretty good idea on how to solve it. I will try to do it this until the end of next week.

kabdelhak added a commit to kabdelhak/OpenModelica that referenced this issue Nov 25, 2020
 - solves ticket OpenModelica#6977
 - new function that takes a context and calls the correct equation create function (for now only jacobian/normal)
 - used inside if equations to actually create jacobian if equations (seems to never have been used because it couldn't work)
 - small fix in backend to have correct size of if equations in advanced adjacency matrices
@kabdelhak
Copy link
Contributor

I created a pull request that solves the issue: #6985
Has to be tested with the testsuite though before i can push it.

The basic problem was: In the backend we created the wrong equation size for if equations in enhanced adjacency matrices so it created too many rows for it -> tearing failed -> entirely untorn system was created -> it still has discrete iteration variables -> fail. (minimalTearing does not depend on enhanced adjacancy matrices so that worked).

After that i encountered an issue with jacobian equation generation because the if-equation handling was not correct since it actually never came to use because of the backend bug. Whoever implemented that in the first place did a good job i only had to change few things to make it possible.

We still get some errors and warnings:

true
"Notification: Automatically loaded package Modelica 3.2.3 due to uses annotation.
Notification: Automatically loaded package Complex 3.2.3 due to uses annotation.
Notification: Automatically loaded package ModelicaServices 3.2.3 due to uses annotation.
"
record SimulationResult
    resultFile = "/home/kab/Repo/FH/promotion/modelicaLibs/ticketTests/6977_tearing_boolean/Physiolibrary/Test/Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm_res.mat",
    simulationOptions = "startTime = 0.0, stopTime = 5.0, numberOfIntervals = 500, tolerance = 1e-06, method = 'dassl', fileNamePrefix = 'Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm', options = '', outputFormat = 'mat', variableFilter = '.*', cflags = '', simflags = ''",
    messages = "LOG_SUCCESS       | info    | The initialization finished successfully without homotopy method.
LOG_SUCCESS       | info    | The simulation finished successfully.
",
    timeFrontend = 0.114956732,
    timeBackend = 0.143585548,
    timeSimCode = 0.025949576,
    timeTemplates = 0.023568243,
    timeCompile = 1.169696841,
    timeSimulation = 0.2609330729999999,
    timeTotal = 1.738809442
end SimulationResult;
"[/home/kab/Repo/FH/promotion/modelicaLibs/ticketTests/6977_tearing_boolean/Physiolibrary/Physiolibrary/Hydraulic.mo:1443:17-1443:163:writable] Warning: In relation Rpp.Resistance == 0.0, == on Real numbers is only allowed inside functions.
[/home/kab/Repo/FH/promotion/modelicaLibs/ticketTests/6977_tearing_boolean/Physiolibrary/Physiolibrary/Hydraulic.mo:1443:17-1443:163:writable] Warning: In relation Rlain.Resistance == 0.0, == on Real numbers is only allowed inside functions.
[/home/kab/Repo/FH/promotion/modelicaLibs/ticketTests/6977_tearing_boolean/Physiolibrary/Physiolibrary/Hydraulic.mo:1443:17-1443:163:writable] Warning: In relation Retha.Resistance == 0.0, == on Real numbers is only allowed inside functions.
[/home/kab/Repo/FH/promotion/modelicaLibs/ticketTests/6977_tearing_boolean/Physiolibrary/Physiolibrary/Hydraulic.mo:1443:17-1443:163:writable] Warning: In relation Rsart.Resistance == 0.0, == on Real numbers is only allowed inside functions.
[/home/kab/Repo/FH/promotion/modelicaLibs/ticketTests/6977_tearing_boolean/Physiolibrary/Physiolibrary/Hydraulic.mo:1443:17-1443:163:writable] Warning: In relation Rsven.Resistance == 0.0, == on Real numbers is only allowed inside functions.
[/home/kab/Repo/FH/promotion/modelicaLibs/ticketTests/6977_tearing_boolean/Physiolibrary/Physiolibrary/Hydraulic.mo:1443:17-1443:163:writable] Warning: In relation Rethv.Resistance == 0.0, == on Real numbers is only allowed inside functions.
[/home/kab/Repo/FH/promotion/modelicaLibs/ticketTests/6977_tearing_boolean/Physiolibrary/Physiolibrary/Hydraulic.mo:1443:17-1443:163:writable] Warning: In relation Rrain.Resistance == 0.0, == on Real numbers is only allowed inside functions.
Error: Internal error - BackendDAEOptimize.makeEquationToResidualExp failed to transform equation: TricuspidValve.open = TricuspidValve.passableVariable > 0.0 to residual form!
Error: Internal error - BackendDAEOptimize.makeEquationToResidualExp failed to transform equation: PulmonaryValve.open = PulmonaryValve.passableVariable > 0.0 to residual form!
Error: Internal error - BackendDAEOptimize.makeEquationToResidualExp failed to transform equation: MitralValve.open = MitralValve.passableVariable > 0.0 to residual form!
Error: Internal error - BackendDAEOptimize.makeEquationToResidualExp failed to transform equation: AorticValve.open = AorticValve.passableVariable > 0.0 to residual form!
Warning: The initial conditions are not fully specified. For more information set -d=initialization. In OMEdit Tools->Options->Simulation->OMCFlags, in OMNotebook call setCommandLineOptions(\"-d=initialization\").
"

But as you can see it overall works. The errors have to be further investigated because that still seems like a tearing bug. I attached the result file if you want to have a look at it and see if it is sensible.

Physiolibrary.Hydraulic.Examples.MeursModel2011.HemodynamicsMeurs_flatNorm_res.zip

perost pushed a commit that referenced this issue Nov 25, 2020
 - solves ticket #6977
 - new function that takes a context and calls the correct equation create function (for now only jacobian/normal)
 - used inside if equations to actually create jacobian if equations (seems to never have been used because it couldn't work)
 - small fix in backend to have correct size of if equations in advanced adjacency matrices
@kabdelhak
Copy link
Contributor

The PR is pushed and did not cause any new issues. If you pull and build the latest version from source you could test this yourself.

kabdelhak added a commit to kabdelhak/OpenModelica that referenced this issue Nov 25, 2020
 - do not count pre(cr) as occurence of cr
 - related to issue OpenModelica#6977
@kabdelhak
Copy link
Contributor

I also fixed the other problem with the errors in #6988 . We basically did not correctly convert the if-equations to if-expressions.

The warnings are valid though. In your case you are checking a parameter so you know what it is gonna be, but to be sure i would always use Modelica.Math.isEqual to compare two Real variables. There might be a case where a user wants to input like Rpp.Resistance = 1e-20 or something like this and you might run into numerical issue. In your case i think it is pretty failsafe but in general i would recommend not using regular equality checks on Real and use the check from MSL instead.

I changed the resistor to be:

model Resistor
  extends Physiolibrary.Hydraulic.Components.Conductor(final Conductance = conditionalConductance);
  parameter Physiolibrary.Types.HydraulicResistance Resistance
    "Hydraulic conductance if useConductanceInput=false";
protected
            final parameter Physiolibrary.Types.HydraulicConductance conditionalConductance = if Modelica.Math.isEqual(Resistance,0) then Modelica.Constants.inf else 1/Resistance;
end Resistor;

And it runs without complains under my newest changes. Note that Modelica.Math.isEqual has an optional third input for the epsilon neighborhood of this check which defaults to 0, so i would change that.

perost pushed a commit that referenced this issue Nov 25, 2020
 - do not count pre(cr) as occurence of cr
 - related to issue #6977
@kabdelhak
Copy link
Contributor

Last PR has also been pushed, as i said everything runs smoothly now. You can build from source or pull the latest nightly build to test it.

Feel free to reopen the ticket if anything still does not work.

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

No branches or pull requests

4 participants