Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
maltelenz opened this issue Mar 27, 2024 · 10 comments
Assignees
Labels
LTX ReSim testing framework Issue that addresses testing framework ref-result Issue addresses the reference results
Milestone

Comments

@maltelenz
Copy link
Contributor

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 maltelenz added the ref-result Issue addresses the reference results label Mar 27, 2024
@maltelenz maltelenz added this to the MSL4.1.0 milestone Mar 27, 2024
@maltelenz
Copy link
Contributor Author

maltelenz commented Mar 27, 2024

Same question for:

ModelicaTest.Media.TestsWithFluid.MediaTestModels.Incompressible.Glycol47
ModelicaTest.Media.TestsWithFluid.MediaTestModels.Water.ConstantPropertyLiquidWater

@beutlich
Copy link
Member

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

@beutlich beutlich changed the title To few variables tested for ModelicaTest.Media.TestsWithFluid.MediaTestModels.Incompressible.Essotherm650? Too few variables tested for ModelicaTest.Media.TestsWithFluid.MediaTestModels.Incompressible.Essotherm650? Mar 28, 2024
@beutlich beutlich added the LTX ReSim testing framework Issue that addresses testing framework label Mar 28, 2024
@MatthiasBSchaefer
Copy link
Contributor

@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
Copy link
Member

beutlich commented Apr 9, 2024

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
Copy link
Member

beutlich commented Apr 9, 2024

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

@MatthiasBSchaefer
Copy link
Contributor

> 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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTX ReSim testing framework Issue that addresses testing framework ref-result Issue addresses the reference results
Projects
None yet
Development

No branches or pull requests

4 participants