lbl-srg / modelica-buildings

Modelica Buildings library
255 stars 158 forks source link

Reference file for model Buildings.DHC.Loads.BaseClasses.Examples.CouplingRCZ1Valve does not match with Dymola 2024 #3881

Open AndreaBartolini opened 5 months ago

AndreaBartolini commented 5 months ago

the reference file used by the OpenModelica CI to check the temperature bui.disFloCoo.senTSup.T of the model Buildings.DHC.Loads.BaseClasses.Examples.CouplingRCZ1Valve has the following trend: image

The same model simulated by using Dymola Version 2024x, 2023-10-06 on Windows 11 pro - 64 bit generates the following trend for the same temperature: image

By the way, the trend generated by Dymola 2024x on Win 11 matches the same generated by OpenModelica v1.24.0-dev-121-gd21af17721 (64-bit), as shown below: image

Can you please check if your official reference file is according to the one generated by Dymola 2024x?

Thanks in advance.

Keep @casella in the loop

JayHuLBL commented 5 months ago

@AndreaBartolini

I run the same model have following results:

Dymola 2024x, on Windows 10:

Screenshot 2024-06-12 at 10 01 43 AM

Dymola 2024x and Dymola 2024xRefresh1, on Ubuntu 22.04, and Dymola 2023x on Ubuntu 20.04

Screenshot 2024-06-12 at 11 05 03 AM

OpenModelica version 1.24.0~dev-30-gdbec042, on Ubuntu 22.04:

re

It shows that the OS difference will cause the results quite different.

However, the reference results should be generated by Dymola 2024x, on Ubuntu 20.04.

casella commented 4 months ago

Is that temperature well-defined, in physical terms? Otherwise, the result would depend on numerical implementation details.

AndreaBartolini commented 4 months ago

The temperature is from the following sensor: image

And herebelow is reported the trend of the flow through the sensor: image

There are several intervals at zero-flow, in that intervals probably the temperature is not well-defined and the result depends on the numerical implementation/approximation/tolerance of the solver.

Probably this scenario is often present in the Buildings library examples, because a lot of hydraulic circuits may work in zero-flow condition.

casella commented 4 months ago

@mwetter this is a completely general problem: if you add these signals to the reference signals for regression testing, you're always going to get in trouble when doing cross-tool validation

If there are no flow reversals through the sensor (as it seems from the simulation results), the other solution is to disallow flow reversal on the sensor, so it always gets the upstream enthalpy via inStream. Maybe that's the best solution in this case. What do you think?

If you agree with that (I think it's already been done in a previous ticket), I guess @AndreaBartolini can collect all these cases and you can fix them in one shot throughout the whole library.

AndreaBartolini commented 4 months ago

Actually the flow reversal is not allowed in the sensor, maybe something should be fixed in the sensor itself: image

AndreaBartolini commented 4 months ago

I'll check it ASAP

mwetter commented 4 months ago

This sensor is configured to use inStream() as it has allowFlowReversal=false, and it has tau=1. Hence the output signal is computed as shown below, where the absolute value is computed using regStep. Unless tau is too small compared to the numerical noise, this should be well behaved.

image

casella commented 4 months ago

@AndreaBartolini please check with the declarative debugger how the sensor output is computed and report.

AndreaBartolini commented 4 months ago

Here the analysis: the derivative of the sensor output is calculated by the following assignment: regular: (assign) der(senTSup.T) := (senTSup.T_b_inflow - senTSup.T) * senTSup.k

and the initial value is given by the following assignment: initial: (assign) senTSup.T := senTSup.T_start

the senTSup.T_b_inflow is calculated by the medium:

(assign) senTSup.T_b_inflow := senTSup.Medium.temperature($cse27)
$cse27 := senTSup.Medium.setState_phX(senTSup.port_a.p, senTSup.port_b.h_outflow, {});

where senTSup.port_b.h_outflow is defined in the modelica model as port_b.h_outflow = inStream(port_a.h_outflow);

that in the running model becomes the following assignment, that involves just the three-way upstream valve val: (assign) senTSup.port_b.h_outflow := bui.disFloCoo.val.vol.dynBal.U / bui.disFloCoo.val.vol.dynBal.m

some additional notes: 1) the port_a of the sensor is connected as following: image

so I suppose that the inStream(port_a.h_outflow) should depend not only from the three-way upstream valve but also from what is connected to the outside connector port_a, maybe that after the back-end simplifications the only dependency is from that valve,

2) the variable bui.disFloCoo.val.vol.dynBal.U has only the initial assignment but it is not clear where is calculated (maybe in a linear system not reported by the debugger...): image

the initial assignment is the following: initial (assign) bui.disFloCoo.val.vol.dynBal.U := bui.disFloCoo.val.vol.dynBal.m * bui.disFloCoo.senTSup.port_b.h_outflow

The only equation that uses it is that has been reported above.

AndreaBartolini commented 4 months ago

@casella let me know if you need more detailed investigation.