Bug in implementation of stream connectors

[rfranke]

Currently the verification fails for some Fluid models that start with zero flow. See e.g. DrumBoiler and RoomCO2. The reason is that OpenModelica adds constants to the generated mixing equations.

Take DrumBoiler as example. OpenModelica generates:

massFlowRate.port_b.h_outflow = DIVISION(0.01680261163051938 + max(qm_S, 1e-07) * evaporator.h_v,
                                         2e-07 + max(qm_S, 1e-07))
temperature.T = someT(..., DIVISION(0.008401305815259689 + max(qm_S, 1e-07) * evaporator.h_v + max(-qm_S, 1e-07) * sink.ports[1].h_outflow,
                                    1e-07 + max(qm_S, 1e-07) + max(-qm_S, 1e-07)) )

This adds an additional spike to massFlowRate.port_b.h_outflow at initial time, while it eliminates the spike for temperature.T and its alias T_S.

The simulation results are correct when removing the constants from the two mixing equations. They should read:

massFlowRate.port_b.h_outflow = DIVISION(max(qm_S, 1e-07) * evaporator.h_v,
                                         max(qm_S, 1e-07))
temperature.T = someT(..., DIVISION(max(qm_S, 1e-07) * evaporator.h_v + max(-qm_S, 1e-07) * sink.ports[1].h_outflow,
                                    max(qm_S, 1e-07) + max(-qm_S, 1e-07)) )

Moreover note that the first equation simplifies to:

massFlowRate.port_b.h_outflow = evaporator.h_v

It appears that the constants stem from the absolute sensors temperature and pressure. They are first added to the mixing equations and then pre-evaluated basing on the attribute m_flow(min=0) in the respective connector. This attribute should be checked earlier, so that the absolute sensors don't appear in in the list of connector elements and in the mixing equations at all.

Alternatively, if the min attributes of the flow variables can't be checked earlier, then the generic max(x, 1e-7) could be replaced with specific positiveMax(x) in the generated equations and expanded later on -- and removed for connector elements with (absolute sensors):

m_flow(min=0)

or (other models -- allowFlowReversal has evaluate=true):

m_flow(min = if allowFlowReversal then -Modelica.Constants.inf else 0).

[adrpo]

I'm assigning this to Per as he implemented the stream connectors handling.
I'm not sure if this regards the back-end or front-end.

[rfranke]

Commit

​https://github.com/OpenModelica/OMCompiler/commit/e18a45ae3291b4723965b8e16b47ab7fd4b20991

introduces the change. I had to adapt 4 tests from MSL 3.2.1 (without affecting the test result):

simulation_libraries_msl32.Modelica.Fluid.Examples.Tanks.TanksWithOverflow.mos
simulation_libraries_msl32.Modelica.Fluid.Examples.HeatingSystem.mos
simulation_libraries_msl32.Modelica.Fluid.Examples.TraceSubstances.RoomCO2WithControls.mos
simulation_libraries_msl32.Modelica.Fluid.Examples.TraceSubstances.RoomCO2.mos

For three further tests the results changed:

openmodelica_cruntime_optimization_benchmark.runDrumBoiler.mos
openmodelica_cruntime_optimization_benchmark.runExReduceDrumBoiler.mos
flattening_libraries_3rdParty_siemens.translateHeatExchanger.mos

@vitalij: Could you have a look?

The verification of RoomCO2 improved from 6 to 2 failures. The remaining 2 seem to be an error in the reference results!?
The model sets each volume.m_flow_turbulent to Modelica.Constants.inf, because no port models are configured. OpenModelica uses 1e60 -- the values I'm used to. The reference results say 3.4e38???

@adrpo: Where do the reference results come from?

[lochel]

It's not good to have wrong test results in our test suite. That makes it horrible to keep developing branches up to date. I think it would be better to deactivate the tests and report them on the trac system.

[rfranke]

You are right. Let's wait for Vitalij to possibly fix the tests. Otherwise I will deactivate them prior to closing this ticket.

Btw. it's also horrible to manually adapt the mos files to the diffs emitted by rtest. Do there exist some best practices -- I could imagine there exists a tool for this work?

[lochel]

You can use rtest -b test.mos or fix-tests.sh listOfFailingTests.txt (both are located in the root testsuite folder).

[adrpo]

@sjoelund.se do we know were the results for RoomCO2 come from?
As far as I know they are build via omc with the results filtered to the variables of interest.

[sjoelund.se]

It might be coming from Dymola for all I know. Or an older MSL version where it had a different value.

[rfranke]

SVN log says that 1.e+60 had been there since the beginning of SVN in 2007. On December 4, 2012 it was moved by sjoelund.se to ModelicaServices.Machine, keeping the value.

[sjoelund.se]

I was referring to that these Fluid functions might have used a different "large" value than the Modelica.Constants value. I know similar things existed in the past.

[rfranke]

According to SVN, rfranke introduced m_flow_turbulent to PartialLumpedVolume on January 15, 2013, including the value of Modelica.Constants.inf if no use_portsData (solving Modelica ticket 736). RoomCO2 had always vessel(use_portsData=false).

[rfranke]

Solved with:

​https://github.com/OpenModelica/OMCompiler/commit/e18a45ae3291b4723965b8e16b47ab7fd4b20991

and

​https://github.com/OpenModelica/OMCompiler/commit/44eef17e379843a61d37663853a174aa4b96a17b

New tickets #3436, #3437 and #3438 have been created for the new issues that showed up.

[adrpo]

The changes seems to have generated a regressions in ThermoPower: ThermoPower.PowerPlants.SteamTurbineGroup.Tests.TestST3LRh_bypass

[adrpo]

The problem was if both lists were empty in function: ConnectUtil.streamSumEquationExp.
I now handled that by not filtering any elements if both of the lists are empty.
@rfranke, maybe we should return 0 in this case? Can you check?
See 6f0b7a70fc84ea54461ad094d3f1fcaf1ef788b5/OMCompiler.

[rfranke]

The inStream value is basically undefined in that case. It would help to get a qualified warning in that case, saying which connection is causing that. Then we could further investigate if the problem is in the model or in the fronend or in the concept.

[adrpo]

@rfranke: we could add a warning for this case. How do you think we should report this? I mean what information should we include in the message?

It seems to happen only if the min/max are zero for all Inside/Outside elements. This appeared in some models inside ThermoPower, Annex60, Buildings and Siemens libraries.

[rfranke]

The warning message should allow to deduce the connection, for instance by listing the involved connectors of inInsideElements and inOutsideElements.

[rfranke]

Meanwhile I investigated the model ThermoPower.PowerPlants.SteamTurbineGroup.Tests.TestST3LRh_bypass.

The problem is in the used turbine model ThermoPower.PowerPlants.SteamTurbineGroup.Examples.ST3L_bypass
that connects for instance:

• ThermoPower.SteamTurbines.Components.BaseReader_water stateHPT_in
• ThermpPower.Water.ValveVap byPassHP
• ThermoPower.Water.ValveVap valveHP

The model BaseReader_water contains the equation

  inlet.h_outflow = inStream(outlet.h_outflow);

for reverse flow conditions. The connected two ValveVap define m_flow(min=0) due to the system wide setting allowFlowReversal=false, i.e. prohibit flow reversal.

The equation in BaseReader_water should be changed to something like:

  inlet.h_outflow = if system.allowFlowReversal then inStream(outlet.h_outflow) else Medium.h_default;

The frontend could emit a warning of the form:

Undefined stream flow equation in steamTurbines.stateHPT_in involving steamTurbines.byPassHP.inlet.h_outflow, steamTurbines.valveHP.inlet.h_outflow

and ideally add the place of the wrong equation: ThermoPower 3.1/PowerPlants.mo line 13664.

Note: the setting allowFlowReversal=false is used to simplify the model -- only half of the stream equations need to be generated and the sizes of equation systems reduce. The warning would help libraries that use this pattern to do it consistently.

Btw. I also cross-checked with Dymola. There steamTurbines.stateHPT_in.inlet.h_outflow evaluates to 3.47 MJ/kg (the value of the inflow of the turbine stage) -- in OpenModelica it is 3.11 (the value of the outflow of the turbine stage) -- showing that the value is really undefined i.e. arbitrary in this case.

[rfranke]

Having thought once more: the warning and the adaptation of the libraries might not be needed when treating the case of empty connection sets as if there was no connection. Then not even a warnig might be needed -- inStream(h_outflow) just evaluates to h_outflow itself in this case.

[rfranke]

fixed in 7c85c2ebbb41f984922caaaa7c83f4c4709e79c4/OMCompiler