MSL 4.1.0 Regressions - Media
he following models fail in result comparison. Tested revision: f9bddf86 (2024-02-16)
Changed models, need reference update after library officer check:
- [ ] ModelicaTest.Media.TestAllProperties.MoistAir
- Reason: err_h_is is included in the reference file, but it shouldn't, it makes no sense to do regression on relative errors, they are just meant for the assert statments. Action: remove
err_T,err_d,err_u,err_h_isfrom thereference.txtfile, see PR #4430. - Updated reference files? - [x] ModelicaTest.Media.TestOnly.R134a_setState_pTX - Reason: the reference file is badly wrong at time = 0.4014, all variables in the reference file have bogus values like -1.8e98. Simulation of the model with Dymola 2024 x and OMC 1.24.0-dev.123 gives no such bogus values. Action: re-generate the files, but then quickly check that the values far from 0.401 are still the same. - Updated reference files? @GallLeo please generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.Air.DryAirNasa - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.Air.MoistAir - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.Air.SimpleAir - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.IdealGases.Air - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.IdealGases.Nitrogen - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.IdealGases.SimpleNaturalGas - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.IdealGases.SimpleNaturalGasFixedComposition - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.LinearFluid.LinearColdWater - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.LinearFluid.LinearWater_pT - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.Water.IdealSteam - Reason: bogus pressure values at 1000101325 Pa, should be 1101325. Not sure how these values managed to survive for so long. Action: re-generate the result files - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.Water.WaterIF97OnePhase_ph - Reason: pipe model in 4.0.0 had dp_nom = 1e9, resulting in very high hydraulic resistance of the pipe (which makes non sense). 4.1.0 has 1e6, which is OK. The change seems reasonable, but we need to figure out when and where it happened to close the issue. Started from this commit e4b68715259b1ce44bff8e4050908f06cee5a9b0 to restore removed models which was then fixed by @hubertus65 in #3918. - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.Water.WaterIF97_ph - Reason: pipe model in 4.0.0 had dp_nom = 1e9, resulting in very high hydraulic resistance of the pipe (which makes non sense). 4.1.0 has 1e6, which is OK. The change seems reasonable, but we need to figure out when and where it happened to close the issue. Started from this commit e4b68715259b1ce44bff8e4050908f06cee5a9b0 to restore removed models which was then fixed by @hubertus65 in #3918. - Updated reference files? @GallLeo plese generate new reference files
- [x] ModelicaTest.Media.TestsWithFluid.MediaTestModels.Water.WaterIF97_pT - Reason: pipe model in 4.0.0 had dp_nom = 1e9, resulting in very high hydraulic resistance of the pipe (which makes non sense). 4.1.0 has 1e6, which is OK. The change seems reasonable, but we need to figure out when and where it happened to close the issue. Started from this commit e4b68715259b1ce44bff8e4050908f06cee5a9b0 to restore removed models which was then fixed by @hubertus65 in #3918. - Updated reference files? @GallLeo plese generate new reference files
Change due to new Dymola version (reference update after library officer check?):
-
[ ] ModelicaTest.Media.TestAllProperties.IncompleteMedia.ReferenceMoistAir - Reason: Verification failures with
err_T,err_d,err_u. Signals likeerr_Tis included in the reference file, but it shouldn't, it makes no sense to do regression on relative errors, they are just meant for the assert statments. Action: removeerr_T,err_d,err_u,err_h_isfrom thereference.txtfile, see PR #4430. - Updated 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
- ModelicaTest.Media.TestAllProperties.MoistAir
I run git bisect on that test model and it reported
cc408ce564d2af08d76a416b434a63bcae61e79e is the first bad commit
The error is only for signal err_h_is now being exactly 0 (while being 1.8189894035458565e-12 before). See #4238.
- ModelicaTest.Media.TestOnly.R134a_setState_pTX
I run git bisect on that test model and it reported
f1574e8116fd5e4d72c5cbae4176f29e1dc3445f is the first bad commit
#3704 is an actual bug fix that I already back-ported to branches maint/3.2.3 (#3790) and maint/4.0.x (#3791) about 3 years ago.
I had a quick look at these, and all of them look to me like "valid" changes: below numerical accuracy, or based on fixes to the test models
As far as I can see, the issue should be closed, all done
There seem to be correctness errors in the report for these?
- ModelicaTest.Media.TestsWithFluid.MediaTestModels.Incompressible.Essotherm650
- ModelicaTest.Media.TestsWithFluid.MediaTestModels.Incompressible.Glycol47
- ModelicaTest.Media.TestsWithFluid.MediaTestModels.Water.ConstantPropertyLiquidWater
Reopening for clarification.
@maltelenz in the report you cite I see all green for those three models. Which errors are you referring to?
This is what I see. The CSVCompare (last column) is red.
Sorry, I was looking at the wrong Essotherm650 😅
As warned about in #4238 , I am seeing (small) regressions for ModelicaTest.Media.TestAllProperties.MoistAir when testing with WSM:
a
reference: 347.024351711 trial: 347.025144721 delta = 7.930095E-04
cp
reference: 1013.31888176 trial: 1013.32351297 delta = 4.631216E-03
cp2
reference: 1013.31888176 trial: 1013.32351297 delta = 4.631216E-03
cv
reference: 724.522935763 trial: 724.526247082 delta = 3.311319E-03
cv2
reference: 724.522935763 trial: 724.526247082 delta = 3.311319E-03
h
reference: 50329.2238194 trial: 50328.5907185 delta = 6.331009E-01
h2
reference: 50329.2238194 trial: 50328.5907185 delta = 6.331009E-01
h_is
reference: 116306.96664 trial: 116306.302865 delta = 6.637751E-01
s
reference: 6924.01316181 trial: 6924.04469876 delta = 3.153695E-02
s_is
reference: 6924.01316181 trial: 6924.04469876 delta = 3.153695E-02
I confirmed that reverting #4238 "fixed" the regression.
I propose that we ask LTX to replace the reference result. @casella, do you agree?
Confirmed, ModelicaTest.Media.TestAllProperties.MoistAir is not yet updated in the v4.1.0 branch of the ref result, see https://github.com/modelica/MAP-LIB_ReferenceResults/tree/v4.1.0/ModelicaTest/Media/TestAllProperties/MoistAir with https://github.com/modelica/MAP-LIB_ReferenceResults/commit/af2c8f2fe0720044d593d560db635321300afe1b being the latest change as of today.
@casella this is waiting for your decision, see my previous comment.
I propose that we ask LTX to replace the reference result. @casella, do you agree?
Sound good to me; even though the difference is neglible for all practical purposes, there is indeed no point at keeping the old one.
@GallLeo, @MatthiasBSchaefer can you please take care of that? Thanks!
All issues settled.