Model Buildings.Fluid.BaseClasses.FlowModels.Validation.BasicFlowFunction_dp_DerivativeCheck2 fails verification

[AndreaBartolini]

The following variables of the model Buildings.Fluid.BaseClasses.FlowModels.Validation.BasicFlowFunction_dp_DerivativeCheck2 fail the verification vs the Dymola reference results:

• err_m_flow

• err_der_m_flow

The maximum error is about 2.9e-7 for err_m_flow and about 9.97e-8 for err_der_m_flow. Both of them are of the same order of magnitude of the tolerance (1e-8) used for the simulation, this is the reason of the discrepancies that essentially depend from the numerical error of the solver and can change by using different compilers and hw/sw architectures.

For example by using the following compilers and architecture I obtain a different result: OpenModelica v1.24.0-dev-183-g504611b14f (64-bit) - builder MinGW - Compiler Clang Dymola 2024x Refresh 1, 2024-04-12 (64-bit) - compiler: Visual Studio 2022/ Visual C++ 2022 (17) Sysop WIN 11 pro (64-bit) Buildings master 745315ed4816b1420de7e5b2d80afd92775db936

In the modelica code both the err_m_flow and the err_der_m_flow are considered acceptable if they are lower than 1e-3, and this is satisfied, maybe that this two variables could be removed from the CI verification.

Keep @casella in the loop.

[casella]

We already had a similar discussion in the MAP-Lib group.

These error figures should be used in the model inside asserts, e.g.assert(abs(err_m_flow)<1e-5, "err_m_flow is too large"), so that if something goes wrong the simulation actually fails because of assertion violation. This is actually already done in the library.

Posting reference trajectories for such integration errors is not really a good idea, because they are by definition tool-dependent (hence difficult to reproduce) and very near to zero, so their relative errors are not well defined and can easily cause false positives.

@mwetter, my recommendation is thus to remove such signal (and all similar signals in the other models of the same sub-package). Possible regressions will be caught by the asserts anyway.

[mwetter]

I agree that the error trajectories should be removed from the reference results, and just check in an assertion.

@hcasperfu : Can you please correct this in the IBPSA library and merge it to Buildings.