ModelicaStandardLibrary icon indicating copy to clipboard operation
ModelicaStandardLibrary copied to clipboard
verified publisher icon modelica

MSL 4.1.0 Regressions - Fluid

Open GallLeo opened this issue 1 year ago • 6 comments

The following models fail in result comparison. Tested revision: f9bddf86 (2024-02-16)

Changed models, need reference update after library officer check:

  • [x] Modelica.Fluid.Examples.BranchingDynamicPipes - Reason: some fixes were made to the pipe models regarding friction and energy conservation that may slightly modify sharp transients such as this one. During the transient, the CSV-compare time-shift tolerance is quite large, so the difference is not caught, but it is when the peak reaches the maximum, see figure below immagine. The new trajectory (in green) has a slightly higher overshoot, which just barely goes outside the 0.002 relative tolerance tube: immagine. - Action: We see no reason to believe that the old reference is better than the new one, so we should just update it. - Updated reference files? @GallLeo please update reference file

  • [x] Modelica.Fluid.Examples.Explanatory.MomentumBalanceFittings - Reason: the issue is described extensively in #3656, suddenExpansion1.m_flow has two solutions and the reference file held the wrong one. Issue fixed in #3657 by adding a suitable start value.
    - Updated reference files? @GallLeo please update the reference file

  • [x] ModelicaTest.Fluid.TestComponents.Fittings.TestMultiPort - Reason: no more regression, no need to do anything. - Updated reference files? Not required.

  • [x] ModelicaTest.Fluid.TestComponents.Fittings.TestJunctionTraceSubstances - Reason: the variable junction1.mC_scaled, which refers to the mass of a trace component in a junction model with mass storage, has a very modest drift compared to the reference trajectory, about 0.003 max relative error, whereas the tolerance tubes are placed at +/-0.002 relative error. Doesn't look like a relevant regression. It is a bit weird that the comparison signal junction1.mC_scaled is a protected variable, we should use junction1.port_2.C_outflow[1] instead, which is public. This is done in #4427, should be also ported to maint/4.1.0 - Updated reference files? @GallLeo please update reference files

  • [x] ModelicaTest.Fluid.TestComponents.Fittings.TestSharpEdgedOrifice - Reason: There was a bug in the Sharp Edged Orifice model that was fixed in #3944, so we need new reference results with the corrected model - Updated reference files? @GallLeo please generate reference files

  • [x] ModelicaTest.Fluid.TestComponents.Pipes.DynamicPipesWithTraceSubstances - Reason: Due to some issue in the pipe model, the last temperature of pipe6 was slightly different from the other ones. After #3953 was fixed, the temperatures are all the same, which looks reasonable. So, we should update the reference files - Updated reference files? @GallLeo please update the reference files.

  • [x] ModelicaTest.Fluid.TestComponents.Sensors.TestPressure - Reason: The test now passes, no need for action. - Updated reference files? Not needed.

  • [x] ModelicaTest.Fluid.TestComponents.Sources.TestSources - Reason: The test now passes, no need for action. - Updated reference files? Not needed.

  • [x] ModelicaTest.Fluid.TestComponents.Valves.TestCheckValve - Reason: The test now passes, no need for action. - Updated reference files? Not needed.

  • [x] ModelicaTest.Fluid.TestComponents.Valves.TestValveCompressible - Reason: The test now passes, no need for action. - Updated reference files? Not needed.

  • [x] ModelicaTest.Fluid.TestComponents.Valves.TestValveDiscrete - Reason: The test now passes, no need for action. - Updated reference files? Not needed.

  • [x] ModelicaTest.Fluid.TestComponents.Valves.TestValveLinear - Reason: The test now passes, no need for action. - Updated reference files? Not needed.

  • [x] ModelicaTest.Fluid.TestComponents.Valves.TestValvesCompressibleReverse - Reason: The reference trajectories have a glitch around the point in time where the flow through V3 changes the sign. The new results are smooth and are definitely more correct, so they should be used - Updated reference files? @GallLeo please generate reference file.

  • [x] ModelicaTest.Fluid.TestComponents.Valves.TestValvesIncompressibleReverse - Reason: The test now passes, no need for action. - Updated reference files? Not needed.

  • [x] ModelicaTest.Fluid.TestComponents.Valves.TestValveVaporizing - Reason: The test now passes, no need for action. - Updated reference files? Not needed.

  • [x] ModelicaTest.Fluid.TestExamplesVariants.BranchingDynamicPipes_MomentumSteadyState - Reason: Slightly changed temperatures after the fix in #3953, new results should be more accurate. - Updated reference files? @GallLeo please update reference files

  • [x] ModelicaTest.Fluid.TestUtilities.Test02RegFun3 - Reason: The test now passes, no need for action. - Updated reference files? Not needed.

  • [x] Modelica.Fluid.Examples.AST_BatchPlant.BatchPlant_StandardWater - Reason: Same as with Modelica.Fluid.Examples.BranchingDynamicPipes: very small difference in pressure spike, probably due to changes in the pipe model. immagine immagine - Action: update the reference file - Updated reference files? @GallLeo please update the reference files

  • [ ] Release notes check: Are all classes mentioned which could lead to result changes in user models?

Useful Links

Current comparison report: https://www.ltx.de/download/MA/Compare_MSL_v4.1.0/comparison_report_overview.html -> Reference result test -> Comparison

Comparison signal definitions: https://github.com/modelica/ModelicaStandardLibrary/tree/master/Modelica/Resources/Reference/Modelica https://github.com/modelica/ModelicaStandardLibrary/tree/master/ModelicaTest/Resources/Reference/ModelicaTest

Reference results: https://github.com/modelica/MAP-LIB_ReferenceResults

GallLeo avatar Feb 27 '24 08:02 GallLeo

Backported #4427 to maintenance branch by #4449

Esther-Devakirubai avatar Aug 15 '24 15:08 Esther-Devakirubai

Currently simulation fails for: ModelicaTest.Fluid.TestComponents.Vessels.TestSimpleTank

Maybe also because of tighter tolerance? Did not investigate, yet.

Failed condition: tank.medium.T >= 272.15 and tank.medium.T <= 403.15

See simulation log and setup, here: https://www.ltx.de/download/MA/Compare_MSL_v4.1.0/Testruns/Dymola/ModelicaTest/_report/ModelicaTest.Fluid.TestComponents.Vessels.TestSimpleTank/testcase_report.html

GallLeo avatar Oct 20 '24 09:10 GallLeo

ModelicaTest.Fluid.TestComponents.Fittings.TestJunctionTraceSubstances did not indicate " @GallLeo please generate reference file."

But, according to the analysis, I also updated the reference. Is that OK @casella?

GallLeo avatar Oct 20 '24 15:10 GallLeo

ModelicaTest.Fluid.TestComponents.Fittings.TestJunctionTraceSubstances did not indicate " @GallLeo please generate reference file."

But, according to the analysis, I also updated the reference. Is that OK @casella?

Yes, we obviously overlooked that. I edited the analysis adding the request a posteriori. Thanks!

casella avatar Oct 20 '24 22:10 casella

Currently simulation fails for: ModelicaTest.Fluid.TestComponents.Vessels.TestSimpleTank

This also happens with OpenModelica, see simulation log.

Failed condition: tank.medium.T >= 272.15 and tank.medium.T <= 403.15

Same for OMC

See simulation log and setup, here: https://www.ltx.de/download/MA/Compare_MSL_v4.1.0/Testruns/Dymola/ModelicaTest/_report/ModelicaTest.Fluid.TestComponents.Vessels.TestSimpleTank/testcase_report.html

The link is unfortunately broken, this is what I get immagine

casella avatar Oct 20 '24 22:10 casella

We also have issues with Modelica.Fluid.Examples.TraceSubstances.RoomCO2WithControls. This has been known for a while, see #4312.

casella avatar Oct 20 '24 23:10 casella

I believe everything that remains is covered in other issues. Feel free to reopen if anyone disagrees.

Closing.

maltelenz avatar Feb 11 '25 12:02 maltelenz