modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
479 stars 169 forks source link

Too few variables tested for ModelicaTest.Media.TestsWithFluid.MediaTestModels.Incompressible.Essotherm650? #4367

Open maltelenz opened 7 months ago

maltelenz commented 7 months ago

In this report https://www.ltx.de/download/MA/Compare_MSL_v4.1.0/Compare/ModelicaTest/testrun_report.html I see:

image

for the model ModelicaTest.Media.TestsWithFluid.MediaTestModels.Incompressible.Essotherm650,

where in the last two columns it says 0/1. I'm interpreting that as "one variable was tested and none was incorrect".

However, the model has 32 comparison signals in both the library and the reference result repository. Am I misunderstanding something?

maltelenz commented 7 months ago

Same question for:

ModelicaTest.Media.TestsWithFluid.MediaTestModels.Incompressible.Glycol47
ModelicaTest.Media.TestsWithFluid.MediaTestModels.Water.ConstantPropertyLiquidWater
beutlich commented 7 months ago

In a similar pattern: ModelicaTest.Media.TestAllProperties.IncompleteMedia.ReferenceMoistAir prints 1 invalid signal out of 27 on the overview page, but it actually is 3 out of 27 when you open the comparison report.

grafik

grafik

MatthiasBSchaefer commented 7 months ago

@beutlich The problem of "false" counting in ModelicaTest.Media.TestAllProperties.IncompleteMedia.ReferenceMoistAir is a problem of CSVCompare: If you compare this with the --comparisonflag normally the ". Failed values:" are listed in the resulting log-file. But in this case this list is empty. So our testing tool (without parsing the html) only knows that it is failed, but not how many. So "1" is the default value for this case, since it's better than counting the empty list of "failed values".

beutlich commented 7 months ago

I am afraid I cannot confirm your observations, i.e., the Failed values are listed as expected. Find my log files attached: ReferenceMoistAir.zip. You can see that I obtain different values for the comparison.

beutlich commented 7 months ago

@MatthiasBSchaefer csv-compare v2.0.4 fixed the need for counting failed values by printing the total number.

MatthiasBSchaefer commented 7 months ago
> In this report https://www.ltx.de/download/MA/Compare_MSL_v4.1.0/Compare/ModelicaTest/testrun_report.html I see:
> 
> ![image](https://private-user-images.githubusercontent.com/329963/317224021-ac0a7ad2-97aa-4c30-98d6-b17450e8be22.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTI3Mjg4MDgsIm5iZiI6MTcxMjcyODUwOCwicGF0aCI6Ii8zMjk5NjMvMzE3MjI0MDIxLWFjMGE3YWQyLTk3YWEtNGMzMC05OGQ2LWIxNzQ1MGU4YmUyMi5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNDEwJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDQxMFQwNTU1MDhaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT01MDZhYTcxNTU1MzIxODY5Mjc1ZmM1YWU0N2JjNDFiNGM0ZDE5MjNkYTMxMzk2MTRmNzQ4NDRkY2RlOTIwYzA4JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.sg3Li8Bj6zKqeMKIBXoEG_ROTFJ6EUMWcxzYqnVyvP8)
> 
> for the model `ModelicaTest.Media.TestsWithFluid.MediaTestModels.Incompressible.Essotherm650`,
> 
> where in the last two columns it says `0/1`. I'm interpreting that as "one variable was tested and none was incorrect".
> 
> However, the model has `32` comparison signals in both [the library](https://github.com/modelica/ModelicaStandardLibrary/blob/maint/4.1.x/ModelicaTest/Resources/Reference/ModelicaTest/Media/TestsWithFluid/MediaTestModels/Incompressible/Essotherm650/comparisonSignals.txt) and the [reference result](https://github.com/modelica/MAP-LIB_ReferenceResults/blob/v4.1.0/ModelicaTest/Media/TestsWithFluid/MediaTestModels/Incompressible/Essotherm650/comparisonSignals.txt) repository. Am I misunderstanding something?

This is due to the smart heuristics in out testing tool. We are first looking for common state variables in both result files (since these are the "most important" ones and for large models we don't want to compare all variables). In the Essotherm650 example we have the state volumes.medium.T in both files.

Only if there are no common states, we take all variables appearing in both result files.

For the general case, this is a smart way to handle, but we are thinking about some exception for MSL-testing.

maltelenz commented 7 months ago

Just to put down explicitly what we talked about during the MAP-LIB meeting yesterday:

For this MSL regression testing, if the set of variables in the reference is not exactly the same set of variables as in the comparison signals file, this should be considered a hard error.

maltelenz commented 7 months ago

this won't be the case in our testing tool, since it's not only for MSL-regression tests.

That's unfortunate, and not the impression I got from @GallLeo during the meeting yesterday?

beutlich commented 7 months ago

Please use an other regression testing tool if you need this !

For the record, these are the existing OSS testing frameworks I am aware of:

beutlich commented 2 months ago

@MatthiasBSchaefer csv-compare v2.0.4 fixed the need for counting failed values by printing the total number.

@MatthiasBSchaefer I wonder if LTX ReSim uses csv-compare v2.0.4. It' closed source and I never got any confirmation if v2.0.4 actually improved the automatic workflows.