#3885 closed defect (fixed)
inStream and Evaluate Parameter
Reported by: | Vitalij Ruge | Owned by: | Vitalij Ruge |
---|---|---|---|
Priority: | blocker | Milestone: | 1.13.0 |
Component: | Backend | Version: | |
Keywords: | Cc: | Rüdiger Franke, Francesco Casella, Willi Braun, Volker Waurich, Per Östlund |
Description (last modified by )
we should transform inStream()
after evaluate parameter and removeSimpleEquation
e.g. in Modelica.Fluid.Examples.Explanatory.MomentumBalanceFittings
we find
rightAdaptor.m_flow:VARIABLE(min = if rightAdaptor.allowFlowReversal then -9.999999999999999e+59 else 0.0 max = 100000.0 start = rightAdaptor.m_flow_start unit = "kg/s" ) "Mass flow rate in design flow direction" type: Real
so we need evaluate rightAdaptor.allowFlowReversal
and then expand inStream()
see #3430 for more detials.
Attachments (3)
Change History (37)
comment:1 by , 9 years ago
Description: | modified (diff) |
---|---|
Summary: | inStream and Eval Parameter → inStream and Evaluate Parameter |
comment:2 by , 9 years ago
Cc: | added |
---|
comment:3 by , 9 years ago
That would be possible I guess, but it would mean that we'd have to keep all the connection information until the backend. But I don't see what the issue is, why can't you evaluate rightAdaptor.allowFlowReversal as it is?
comment:4 by , 9 years ago
How I understand inStream()
reduce his neighborhood depend on min/max
. So if we transform inStream()
in the sum before merge alias or evaluate constant/parameter
e.g. min =if rightAdaptor.allowFlowReversal then -9....
the reducation of the neighborhood failed.
comment:6 by , 9 years ago
Replying to perost:
I'm sorry, I don't know what you mean by neighbourhood.
Perhaps, I'm wrong. I mean something like the fixes from Rüdiger
see
treat_reductions_to_empty_connection sets special isZeroFlow
-check.
comment:7 by , 9 years ago
There some notes allowFlowReversal #3430.
I'm using +d=dumpdaelow
and find in the dump min =if rightAdaptor.allowFlowReversal then -9.
...
Additional we have min/max
values just after removeSimpleEquation.
In the dump we don't have anymore inStream()
so we should transnform inStream()
in something like inStreamBackend()
?!
follow-up: 10 comment:8 by , 9 years ago
Ok, I see. We use min/max to simplify inStream, but those might not be evaluated until the backend. So the real issue is really the whole structural parameter confusion.
I don't really like the idea of moving the evaluation of inStream to the backend, for memory and efficiency reasons. But I'm no expert on stream connectors, I just implemented them according to the spec. Maybe Rüdiger or someone else has some input on this. Or maybe we should discuss it on the meeting tomorrow.
comment:9 by , 9 years ago
I hope we don't need evlauateinStream()
. e.g. inStream()
transform in
inStreamBackend(list1, list2, eps)
which we can link agian an inline C/C++
inStreamCode(array1, array2, n, m, eps)
(like semiLinear)
function.
So we need to add new derivation
rule and reduce lis1 and list2 after removeSimpleEquation...
Maybe Rüdiger or someone else has some input on this. Or maybe we should discuss it on the meeting tomorrow
good idea :)
comment:10 by , 7 years ago
Component: | *unknown* → Frontend |
---|---|
Milestone: | Future → 2.0.0 |
Owner: | changed from | to
Status: | new → assigned |
Replying to perost:
Ok, I see. We use min/max to simplify inStream, but those might not be evaluated until the backend. So the real issue is really the whole structural parameter confusion.
I don't really like the idea of moving the evaluation of inStream to the backend, for memory and efficiency reasons. But I'm no expert on stream connectors, I just implemented them according to the spec. Maybe Rüdiger or someone else has some input on this. Or maybe we should discuss it on the meeting tomorrow.
Here is the required input.
In general, flow reversal can occur, so for connection among more than two stream connectors, inStream() results in a fairly elaborate expression, including conditional expressions. If, however, it can be determined a priori a that a fluid will never flow out of a certain connector (i.e., its flow variable has min = 0), then the corresponding term can be removed for good from the inStream() expression. This can lead to very drastic simplifications in the model structure.
Now, in many libraries (including Modelica.Fluid and ThermoPower), the choice whether or not flow reversal can happen at a certain connector is not fixed, but it depends on some parameter (e.g., Boolean allowFlowReversal
. The typical pattern is
FluidPort port(m_flow(min = if allowFlowReversal then Modelica.Constant.inf else 0.0));
From this point of view, all parameters that determine the min attribute of flow variables of stream connectors are structural parameters, and they should be evaluated as early as possible, before BLT analysis and code generation. On the other hand, note that these min attributes can only influence the actual expressions of inStream() operators and their dependencies on other variables.
@perost, can you deal with this as with all other structural parameters, i.e., make sure that those min attributes are evaluated by the front end before computing the inStream() expressions?
comment:11 by , 7 years ago
Note: some motivation to do simplification for inStream
in Backend was:
1) many paramter
allowFlowReversal
has(ot user can help with) an annotion(Evaluate=True)
2) Only after removeSimpleEquation we can find min,max value for some variable and finally simplify const streams like in #4441 to zero and notposetiveMax(.) = max(0,eps) = eps
follow-up: 15 comment:12 by , 7 years ago
Proposal:
inStream
interduce -> posetiveMax(cref, eps) and posetiveMax(-cref, eps)- simpliy posetiveMax in Backend after removeSimpliEquation and EvaluateParameter and before IndexREduction
- posetiveMax(cref, eps) = cref where variable is constant >= 0
- posetiveMax(-cref, eps) = cref where variable is constant <= 0
- posetiveMax(cref, eps) = 0 if variable(cref).min >= 0
- posetiveMax(-cref, eps) = 0 if variable(cref).max <= 0
comment:14 by , 7 years ago
Milestone: | 2.0.0 → 1.13.0 |
---|
comment:15 by , 6 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Replying to vitalij:
Proposal:
inStream
introduce -> positiveMax(cref, eps) and positiveMax(-cref, eps)- simpliy positiveMax in Backend after removeSimpliEquation and EvaluateParameter and before IndexREduction
- positiveMax(cref, eps) = cref where variable is constant >= 0
- positiveMax(-cref, eps) = cref where variable is constant <= 0
- positiveMax(cref, eps) = 0 if variable(cref).min >= 0
- positiveMax(-cref, eps) = 0 if variable(cref).max <= 0
There are some errors here, from what I understand it should be
- positiveMax(-cref, eps) = -cref where variable is constant <= 0
- positiveMax(cref, eps) = 0 if variable(cref).max <= 0
- positiveMax(-cref, eps) = 0 if variable(cref).min >= 0
comment:16 by , 6 years ago
Component: | Frontend → Backend |
---|---|
Owner: | changed from | to
Priority: | critical → blocker |
Status: | reopened → assigned |
comment:17 by , 6 years ago
I actually got a wrong, non-physical behaviour in a customer model that I cannot post here for confidentiality reasons. Detailed analysis of the -d=optdaedump
output turned out to be consistent with the implementation proposed in comment:12, which I think is not correct.
I guess #PR2319 should be amended to reflect the definitions in comment:15.
comment:18 by , 6 years ago
Status: | assigned → accepted |
---|
comment:19 by , 6 years ago
Regarding the topic of min/max merging of alias variables involved in the inStream computation, see #5099
follow-up: 23 comment:21 by , 6 years ago
I prepared some tests for stream connectors that could be added to the testsuite, see the attached package. I ran them all with -d=optdaedump
and analyzed how some stream variables are computed. I reported the analysis in the documentation layer of the test cases. The good news is that the optdaedump output is always correct and consistent with my analysis. The bad news is that in all these cases the front-end actually manages the simplification, so that the equations passed to the back-end are actually not amenable to further manipulation by simplifyInStream
, so these test cases are of no use to check the correct behaviour of simplifyInStream
.
This is consistent with the fact that when I tried to reproduce the bug in our customer models by removing unnecessary details, we got the correct results. Apparently, only in a handful of more complex cases (which is hard to reproduce) the front-end fails to introduce the appropriate simplifications for some reason, and only then the simplifyInStream
function of the back-end kicks in.
@perost can you shed some light as to when the front-end (both old and new) can manage the simplifications in inStream due to min>=0 flow variables and when it cannot?
comment:22 by , 6 years ago
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
In the meantime, I checked our customer's model with the latest nightly after @vitalij's commits, and the results are now correct. Thanks Vitalij!
It would be good if we could have some more test cases for simplifyInStream
, but that's beyond the scope of this ticket.
by , 6 years ago
Attachment: | TestStreamConnectors.mo added |
---|
follow-up: 24 comment:23 by , 6 years ago
Replying to casella:
@perost can you shed some light as to when the front-end (both old and new) can manage the simplifications in inStream due to min>=0 flow variables and when it cannot?
I don't really understand the question, but both frontends remove stream connectors with zero flow before generating inStream equations (which for outside connectors means max == 0
and for inside connectors min == 0
). They don't do any constant evaluation, so they rely on the min/max attributes having been evaluated in an earlier phase. The new frontend evaluates those attributes if they have constant bindings (i.e. is constant or structural parameter), the old frontend I think tries its best to always evaluate them.
comment:24 by , 6 years ago
Replying to perost:
Replying to casella:
@perost can you shed some light as to when the front-end (both old and new) can manage the simplifications in inStream due to min>=0 flow variables and when it cannot?
I don't really understand the question,
In fact you gave me the required answer :)
but both frontends remove stream connectors with zero flow before generating inStream equations (which for outside connectors means
max == 0
and for inside connectorsmin == 0
). They don't do any constant evaluation, so they rely on the min/max attributes having been evaluated in an earlier phase.
Which may or may not be the case. Many libraries (e.g. Modelica.Fluid, ThermoPower, as well as the tests posted here) set the min attribute based on some parameter expression, which requires some evaluation.
The new frontend evaluates those attributes if they have constant bindings (i.e. is constant or structural parameter), the old frontend I think tries its best to always evaluate them.
OK, that's consistent with what I experienced. The old front end managed to carry out the symbolic simplifications in all the 10 cases I tried.
Now I removed the actualStream part of the model (see second attachment TestStreamConnectorsNoActualStream) to run the tests with -d=newInst and now I see the $OMC$PositiveMax macros in the dump of the first stages of the backend. They disappear as expected after simplifyInStream
is executed.
comment:25 by , 6 years ago
The simplifications in tests Test1
through Test5
of TestInStreamNoActualStream.Tests
, which contain unconnected connectors, one-to-one connections and a three way connection with a port with min = 0, are handled by the new front end correctly, as expected based on comment:23.
comment:26 by , 6 years ago
TestStreamConnectorsNoActualStream.Tests.Test6
has a component with this connector declaration
Interfaces.Flange inlet(m_flow(min = if allowFlowReversal then -1e9 else 0))
where allowFlowReversal is a parameter which is false by default. The new FE doesn't do any evaluation, and passes
pipe1.inlet.h_outflow = ($OMC$PositiveMax(-pipe2.outlet.m_flow, 1e-007) * pipe2.outlet.h_outflow + $OMC$PositiveMax(-pipe3.inlet.m_flow, 1e-007) * pipe3.inlet.h_outflow) / ($OMC$PositiveMax(-pipe2.outlet.m_flow, 1e-007) + $OMC$PositiveMax(-pipe3.inlet.m_flow, 1e-007))
to the back-end, where simplifyInStream
turns it to
pipe1.inlet.h_outflow = (max(pipe2.inlet.m_flow, 1e-007) * source2.h + max(-pipe3.inlet.m_flow, 1e-007) * sink.h) / (max(pipe2.inlet.m_flow, 1e-007) + max(-pipe3.inlet.m_flow, 1e-007))
which doesn't take into account the fact that pipe3.inlet.m_flow.min
can be evaluated to zero because pipe3.allowFlowReversal = false
, that allows to simplify the expression to
pipe1.inlet.h_outflow = source2.flange.h_outflow = source2.flange.h
which is in fact the output of the old front-end.
The optdaedump reports
pipe3.inlet.m_flow:VARIABLE(flow=true min = if pipe3.allowFlowReversal then -1000000000.0 else 0.0 ) type: Real
so apparently simplifyInStream
does not evaluate the min attribute and therefore does not recognize that min >=0, which would allow a further simplification.
Of course one could set annotation(Evaluate = true)
on allowFlowReversal
, so that the front-end does the processing. On the other hand people writing models may fail to recognize that this is necessary for the optimization to take place - the specification doesn't mention the need of setting Evaluate = true.
Since the removal of terms made irrelevant by min = 0 is more of an optimization, I am ok with performing the simplification in the back-end in this case. However, simplifyInStream
should constant evaluate the min and max expressions, otherwise the thing doesn't work.
@vitalij, can you take care of this?
Thanks!
by , 6 years ago
Attachment: | TestStreamConnectorsNoActualStream.mo added |
---|
comment:27 by , 6 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:28 by , 6 years ago
simplifyInStream
don't force the evaluation of parameters.
We using simplify to evaluate the min/max values
https://github.com/OpenModelica/OMCompiler/blob/master/Compiler/BackEnd/BackendDAEOptimize.mo#L244
I would adress the topic evaluate parameter in some other module before call simplifyInStream
e.g. EvalutateFinalParameters
with some additional flag e.g. forceEvalutionFlowAttributes or something else.
Note there maybe an site effect, because
-> simplifyInStream
is after removeSimpleEquation
(need alias merge)
-> maybe simplifyInStream
can introduce new alias equation
-> simuliation/optimization/jacobian backend has an additional removeSimpleEquation
but initialization-backend don't have, which maybe need most of all the attributes (like start) from the alias variables.
comment:29 by , 6 years ago
Of the other hand if you thinking about a heuristic like for tearing set where OM replace the parameter it's would maybe be possible direct if we can use the same function. But I like the solution with EvaluateParameter module more.
comment:30 by , 6 years ago
Simplify other Modelica tools like Dymola
pipe1.inlet.h_outflow = source2.flange.h_outflow = source2.flange.h
comment:31 by , 6 years ago
I checked Modelica.Fluid.Interfaces.PartialTwoPort
, and indeed the base class for two-port components contains the Evaluate annotation:
parameter Boolean allowFlowReversal = system.allowFlowReversal "= true to allow flow reversal, false restricts to design direction (port_a -> port_b)" annotation(Dialog(tab="Assumptions"), Evaluate=true); Modelica.Fluid.Interfaces.FluidPort_a port_a( redeclare package Medium = Medium, m_flow(min=if allowFlowReversal then -Constants.inf else 0))
Since the MSL is meant as the standard reference for modelling style, I would say we can safely assume that if one wants this kind of optimization, he needs to tell the compiler explicitly to evaluate the parameter.
At this point, based on comment:23, most simplifications will be handled by the new front-end, because the min/max expressions will be constant evaluated.
The simplifyInStream function would then just be needed to handle those cases where the flow is set to zero without an explicit min = 0
declaration, as well as those where min/max propagation would be needed, for example if the min attribute is not set on the port variable but on an alias variable for the flow rate, or if there are two or more series-connected pipes and only one has allowFlowReversal=false
, etc. In this case alias elimination and min/max merging is required to perform the simplification, which is a job for the back-end, not for the front-end.
I modified the test suite to include the Evaluate
annotation on the allowFlowReversal
parameter, see the attached TestStreamConnectorsNoActualStreamEvaluateParams.mo
. I ran all the previous tests with the new front-end and they work as expected.
I then added one more Test11
with two series-connected pipes, of which the one involved in the three-way connection has allowFlowReversal=true
, but the series-connected one has allowFlowReversal=false
. In this case, as expected, the new front end does not perform any simplification, because it only looks at the min attribute of the connector involved in the three-way connection set, while simplifyInStream
does it, by considering the limitation on the series-connected pipe via alias elimination and min/max merging.
Summing up, if one sets Evaluate = true
on the parameters involved in the min/max attributes, the current situation is perfectly fine both with the old and new front-end. I will then re-close the ticket.
@perost, it would be nice if you added the TestStreamConnectorsNoActualStreamEvaluateParams.mo
tests to the OMC testsuite. They include some asserts that check that critical stream variables are computed as expected. Can you please take care of that?
by , 6 years ago
Attachment: | TestStreamConnectorsNoActualStreamEvaluateParams.mo added |
---|
comment:32 by , 6 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:33 by , 6 years ago
@perost,@casella I will add TestStreamConnectorsNoActualStreamEvaluateParams.mo as tests for simulation with +d=newInst
to minimize the risk that I will broke something if I try to fix #5104
comment:34 by , 6 years ago
Fine for me. Let's just make sure that once newInst becomes the default and -d=newInst
is no longer needed, these tests are still performed.
Added perost to CC as he implemented inStream/actualStream operators.