The frontends do not handle actualStream correctly

[Francesco Casella]

Please flatten the attached TestActualStream model.

The ​PartialPump models contains:

Hb_flow = port_a.m_flow*actualStream(port_a.h_outflow) +
          port_b.m_flow*actualStream(port_b.h_outflow);

When flattening these equations, the OF returns

pump.Hb_flow = 
  pump.port_a.m_flow * smooth(0, pipe.port_b.h_outflow) +
  pump.port_b.m_flow * smooth(0, pump.port_b.h_outflow);

while the NF returns

pump.Hb_flow = 
  pump.port_a.m_flow * smooth(0, pipe.port_b.h_outflow) +
  pump.port_b.m_flow * smooth(0, if pump.port_b.m_flow > 0.0
                                 then sink.ports[1].h_outflow
                                 else pump.port_b.h_outflow);

According to ​Section 15.3 of the specification

actualStream(c.h_outflow) = if c.m_flow > 0 
  then inStream(c.h_outflow)
  else c.h_outflow;

Additionally, whenever actualStream is multiplied by its corresponding flow variable, the corresponding expression turns out to be continuous (but not differentiable) even in the presence of a conditional expression, so it should be surrounded by smooth(0, ...).

Notice that if the flow variable (or some of its aliases) has a min = 0 or max = 0, then one of the two branches of the conditional expression will never be triggered, so one can replace the entire conditional expression by the always active branch. At that point, smooth is no longer necessary, because there is no longer any potentially discontinuous expression to declare as actually continuous. Since system.allowFlowReversal = false, these min/max values are indeed set.

So, the output of the NF is wrong for two reasons:

First,

pump.port_b.m_flow * smooth(0, if pump.port_b.m_flow > 0.0
                                 then sink.ports[1].h_outflow
                                 else pump.port_b.h_outflow);

should rather be

smooth(0, pump.port_b.m_flow * (if pump.port_b.m_flow > 0.0
                                 then sink.ports[1].h_outflow
                                 else pump.port_b.h_outflow));

Second, given that pump.port_b.m_flow.max = 0, i.e., water always flows out of the port, likewise that expression can be simplified by only keeping the active branch, which also means an explicit smooth annotation is no longer needed, because the conditional expression is also gone. Hence,

pump.port_b.m_flow*(
  if pump.port_b.m_flow > 0.0 
  then sink.ports[1].h_outflow
  else pump.port_b.h_outflow)

shoud be simplified into

pump.port_b.m_flow*pump.port_b.h_outflow

without any need of a smooth() operator.

If the TestActualStreamFlowReversal model, which has system.allowFlowReversal = false, is flattened, then instead of getting

pump.Hb_flow =
  pump.port_a.m_flow * 
  smooth(0, if pump.port_a.m_flow > 0.0
            then pipe.port_b.h_outflow
            else pump.port_a.h_outflow) +
  pump.port_b.m_flow * 
  smooth(0, if pump.port_b.m_flow > 0.0
            then sink.ports[1].h_outflow
            else pump.port_b.h_outflow);

one should get

pump.Hb_flow =
  smooth(0, pump.port_a.m_flow * 
              if pump.port_a.m_flow > 0.0
              then pipe.port_b.h_outflow
              else pump.port_a.h_outflow) +
  smooth(0, pump.port_b.m_flow * 
              if pump.port_b.m_flow > 0.0
              then sink.ports[1].h_outflow
              else pump.port_b.h_outflow);

Implementing this feature correctly is crucial for the success of a few MSL example cases, so I'd try to fix it in time for 1.14.0. I won't bother fixing the OF, unless it is a trivial fix.

[Per Östlund]

​#470 should fix the issue with the if-expressions not being evaluated. The issue was that the max attribute of the flow value was -(10000.0) when actualStream was evaluated, which wasn't handled by the code checking the flow direction. Now we simplify the expressions first to get rid of such structures.

[Karim Adbdelhak]

Replying to perost:

​#470 should fix the issue with the if-expressions not being evaluated. The issue was that the max attribute of the flow value was -(10000.0) when actualStream was evaluated, which wasn't handled by the code checking the flow direction. Now we simplify the expressions first to get rid of such structures.

This should actually break one model (Modelica.Fluid.Examples.HeatingSystem), but that would be my fault i have to catch that in the backend! If it breaks you can just deactivate the model temporarily and push it anyway, i have to update my check for artificial variables and reactivate it.

[Per Östlund]

Replying to Karim.Abdelhak:

Replying to perost:

​#470 should fix the issue with the if-expressions not being evaluated. The issue was that the max attribute of the flow value was -(10000.0) when actualStream was evaluated, which wasn't handled by the code checking the flow direction. Now we simplify the expressions first to get rid of such structures.

This should actually break one model (Modelica.Fluid.Examples.HeatingSystem), but that would be my fault i have to catch that in the backend! If it breaks you can just deactivate the model temporarily and push it anyway, i have to update my check for artificial variables and reactivate it.

I'd only find out that it breaks after I've pushed the change in anyway. I meant the min attribute above though, not max, and it's of course simplified to -10000.0 in a later phase of the NF.

[Per Östlund]

Possibly fixed in ​#473. I don't know if I've understood everything correctly, but it seems to give the expected result for the two test models at least.

[Karim Adbdelhak]

Replying to perost:

Possibly fixed in ​#473. I don't know if I've understood everything correctly, but it seems to give the expected result for the two test models at least.

Works correctly for the case we looked at. @Francesco please close this ticket if you agree. :)

[Francesco Casella]

The example with the pump looks good.

I also looked at the ​Jenkins report, the improvements are as expected, while the regressions seem irrelevant (e.g. the models failures in Buildings 5.0.0 are running fine in Buildings_latest), or bogus (e.g. the ScalableTestSuite models breaking because of lack of resources).

Mission accomplished :)