Opened 6 years ago
Closed 6 years ago
#5335 closed defect (fixed)
Equation order influences the selection of alias variables
Reported by: | Per Östlund | Owned by: | Lennart Ochel |
---|---|---|---|
Priority: | blocker | Milestone: | 1.14.0 |
Component: | Backend | Version: | v1.14.0-dev-nightly |
Keywords: | Cc: | Willi Braun, Francesco Casella, Karim Adbdelhak |
Description
In the MSL the Modelica.Thermal.FluidHeatFlow.Examples.SimpleCooling model simulates correctly with the old frontend, but not with the new. The Modelica.Thermal.FluidHeatFlow.Examples.IndirectCooling model on the other hand simulates correctly with the new frontend, but not with the old.
When trying to simulate the SimpleCooling
model with the NF it gives an error that a nonlinear system involving ambient2.flowPort.H_flow
couldn't be solved. Looking at the -d=optdaedump
output reveals that pipe.flowPort_b.H_flow
is an alias of -ambient2.flowPort.H_flow
when using the NF, but when using the OF it's instead ambient2.flowPort.H_flow
that's chosen as an alias of -pipe.flowPort_b.H_flow
.
To me it seems like it shouldn't matter which variable is chosen as an alias, but apparently it matters in this case. This might also affect for example Modelica.Mechanics.MultiBody.Examples.Elementary.SpringDamperSystem, which gives the wrong result when simulated using the NF. If the list of equations in the flat model is reversed it suddenly simulates correctly though, but this is obviously not a tenable solution since it will just cause other models to fail instead.
Change History (14)
follow-up: 2 comment:1 by , 6 years ago
follow-up: 8 comment:2 by , 6 years ago
Replying to vitalij:
maybe related to #4270
@vitalij, thanks for the hint!
So, we have potential issues related to the ordering of the equations in all these cases:
- alias selection with different start values (this ticket, but see also #4603, #5252)
- missing initial equations (partially addressed by #4274, if the user notices and adds the missing initial equations or adds fixed = true where appropriate)
- tearing variable selection (see ticket:4270#comment:15)
- state selection when dynamic state selection results from the dummy derivative algorithm (see ticket:4270#comment:15)
comment:3 by , 6 years ago
In this specific case, I guess implementing #4603 may be decisive.
The right choice of start attributes in alias sets is crucial for the success of simulation with nonlinear equations in the initialization phase. Section 8.6.2 of the specification was written precisely for this purpose, because how this choice is made cannot simply be left to the tool, which would end up in a kind of tool-dependent initialization lottery.
I understand that is what is implemented in Dymola, which was also used to build the MSL examples, so if we also implement that, we should be on the right track.
follow-up: 5 comment:4 by , 6 years ago
Replying to casella:
The right choice of start attributes in alias sets is crucial for the success of simulation with nonlinear equations in the initialization phase.
I actually think that all conflicting start values of a model should be fixed by the modeler. We can provide a better solution than just guessing with the confidence numbers, but for a consistent model there should be no conflicts.
There is no way of actually knowing what the modeler intended and if the Backenend guesses wrong it may lead to misinterpretations. Even if the Backend uses the right values to initialize, but the modeler intended other values with which the simulation would fail, it would lead to wrong conclusions.
Please correct me if i don't understand a crucial point here, but i think we should rather focus on better error and conflict reports.
EDIT:
I have looked a bit into the computation of the confidence number and found this comment:
"the value is selected as follows:
- if the exp is a cref with less depth
- for the ones with the minimum depth equal, select one that is non-zero"
So it just depends on the cref depth which i don't think is always the correct choice. We should not count 100% on the confidence number, just if we don't have another choice.
follow-up: 7 comment:5 by , 6 years ago
Replying to Karim.Abdelhak:
Replying to casella:
The right choice of start attributes in alias sets is crucial for the success of simulation with nonlinear equations in the initialization phase.
I actually think that all conflicting start values of a model should be fixed by the modeler. We can provide a better solution than just guessing with the confidence numbers, but for a consistent model there should be no conflicts.
We have been discussing this issue a lot in the past, and I disagree, let me try to elaborate why that is the case.
Equation-based, object-oriented modelling follows a declarative approach: you write the equations, the solver computes the solution. The need to indicate start attributes to prime iterative solvers is a necessary evil whenever nonlinear equations with no closed-form solutions are involved, but it should be kept at the barest possible minimum.
An interesting case showed up in the PowerGrids library I have been working on recently, which is described in this paper. The same Complex voltage appears in the connector, in the port model (whose voltage is set to the connector values by a binding equation), and in the internal variables of the model, which are set to be equal to the port model values. Start values for this voltage are conveniently set in the port model, which is written once and for all, using parameters which are set using the results of a static power flow analysis. All other alias variables have a default start and nominal attribute which is defined in a type stored in the library, which should have lower priority.
I think you agree with me that explicitly setting start attributes manually in the library for all the alias variables is really dumb work, and most importantly it unnecessarily clutters the code to provide an information that can instead be inferred automatically if the appropriate rules are followed. I actually did that in a first version of the library (because I did not trust the implemented priority rules in OMC), but eventually the thing got so ugly that I removed all the stuff, and got back to the original idea of Section 8.6.2.
So, I think that resolving all alias conflicts in the models manually is not the right way to go, and may even be unfeasible (or at least require very ugly code) in some cases. One should set the start (and nominal!) values at one place (with high resulting priority), and then have it automatically propagated to all the other aliases, which should have lower priority default attributes, e.g. coming from Modelica.SIunits
types, or modifications thereof.
The procedure sketched in Section 8.6.2 was devised precisely with this purpose in mind by Sven Erik Mattsson many years ago, successfully implemented in Dymola, and worked satisfactorily in all the use cases I encountered so far that could be critical from this point of view. From the discussion in #4603 and #5252, it seems that the current implementation in OMC is still not fully satisfactory. It may also be the case that the rule needs to be improved, in which case I'd be happy to discuss how.
Summing up, as I see it we need to
comment:6 by , 6 years ago
I had a quick look at the verification failure reports of the models mentioned above, and I saw a lot of the behaviour reported in #5328, see, e.g. body1.v_0[1]
in this report.
I'm not sure this behaviour can be explained by a bad choice of start attributes only. In fact, the solutions seem to start with the right values, but then they behave oddily for a certain amount of time, before taking a sharp turn towards more reasonable results. That's really weird.
@karim, I think you (or someone else in Bielefeld) should have a look at the issue of #5328, since it's really odd and it shows up in a number of places.
In other cases, such as Modelica.Thermal.FluidHeatFlow.Examples.SimpleCooling, the wrong alias selection seems a very likely cause of the initialization failure, see my previous comment on how to fix this.
comment:7 by , 6 years ago
Replying to casella:
We have been discussing this issue a lot in the past, and I disagree, let me try to elaborate why that is the case.
It may also be the case that the rule needs to be improved, in which case I'd be happy to discuss how.
I didn't know that, but i see your arguments. Maybe an improvement for this rule is the way to go, i will look into the issues you posted! :)
comment:8 by , 6 years ago
Replying to casella:
Replying to vitalij:
maybe related to #4270
@vitalij, thanks for the hint!
So, we have potential issues related to the ordering of the equations in all these cases:
The main risen or other motivation for #4270(sorting equations and variables) is not fix the issues with alias and/or tearing. But improve the stability in which one avoid/reduce the side effects for developer which introduced by order. Of course the heuristics can be improved, but per definition is a heuristic and depend from luck. If somebody have implemented a new backend module or new stuffs in the frondend then the issue with the order may occur more often. So if somebody is not familiar with the backend heuristics then the probability is great that somebody will fix it in some other way or giving up.
Of the other hand the module --preOptModules+=sortEqnsVars
work but it's not finished to 100% for this goal.
comment:9 by , 6 years ago
Understood.
In this case, we should make sure that the selection is univocal, and the right one :)
follow-up: 11 comment:10 by , 6 years ago
I looked at Modelica.Thermal.FluidHeatFlow.Examples.SimpleCooling
and saw that a variable with "flow=true" gets kept. Is that expected behaviour?
Locally i changed the alias removal such that flow variables can't be chosen and everything works fine for this model. I'm not deep enough into this to evaluate if we want to avoid keeping flow variables in general. Can someone confirm or deny?
Also we don't seem to have any alias conflicts here, so that cannot be the problem.
comment:11 by , 6 years ago
Replying to Karim.Abdelhak:
I looked at
Modelica.Thermal.FluidHeatFlow.Examples.SimpleCooling
and saw that a variable with "flow=true" gets kept. Is that expected behaviour?
As far a I understand, in general the fact that a variable is or is not a flow variable should be irrelevant w.r.t. the choice of which alias to pick.
Flow variables can only show up in connectors, and modellers usually (but not necessarily!) set meaningful start attributes on internal variables of the model, which are then made equal to the connector variables by some equations. If this practice is followed, then it is a good idea to avoid flow variables when picking the representative variable of an alias set. In fact, one should then avoid all connector variables (also effort, input, output, and stream).
However, one may as well set the start value on the flow variable of the connector and use it directly in equations, e.g.
FluidPort port_a(m_flow = m_flow_start); FluidPort port_b(m_flow = -m_flow_start); equation der(M) = port_a.m_flow - port_b.m_flow;
In this case, ignoring the start value of flow variables m_flow
would be a bad idea.
Locally i changed the alias removal such that flow variables can't be chosen and everything works fine for this model. I'm not deep enough into this to evaluate if we want to avoid keeping flow variables in general. Can someone confirm or deny?
I guess this may have just been a coincidence. Could you please check what were the start attributes of all variables in that alias set and report them?
Also we don't seem to have any alias conflicts here, so that cannot be the problem.
This is not necessarily the case. If the priority computation is not correct (as the analysis in #4603 seems to imply), there may be a conflict that incorrectly went unnoticed.
comment:12 by , 6 years ago
Milestone: | 2.0.0 → 1.14.0 |
---|
comment:13 by , 6 years ago
I tried simulating Modelica.Thermal.FluidHeatFlow.Examples.SimpleCooling
with and without -d=newInst
and as it seems the results are the same (also it does not crash).
For Modelica.Thermal.FluidHeatFlow.Examples.IndirectCooling
only without -d=newInst
seems to crash, but as we want to get everything running for the NF i don't think we should invest more into fixing that, if it is not a general problem.
Can someone confirm my results? Is this still a blocker for 1.14.0 as is?
EDIT: Maybe this results from the changes from #4793
comment:14 by , 6 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Based on the Jenkins report, all FluidHeatFlow
models pass verification when using the NF. I don't care much about results with the OF in this case.
So I guess we can close this ticket.
maybe related to #4270