Open maltelenz opened 8 months ago
Duplicate of #4349 (does not matter if new or altered models) and already resolved in https://github.com/beutlich/MAP-LIB_ReferenceResults/blob/v4.1.0/ModelicaTest/MultiBody/Forces/EngineGasForce/EngineGasForce.csv where gasForce.dens no longer is listed in the simulation result CSV file. Can you live with Dymola 2014x reference results being available in https://github.com/beutlich/MAP-LIB_ReferenceResults?
Not a duplicate, the other issue is about new models.
I'm not satisfied with the fully regenerated baselines on your fork, maplib decided for higher ambitions this time, to only update references where we have a reason.
Not a duplicate, the other issue is about new models.
It is a duplicate in the sense that https://github.com/beutlich/MAP-LIB_ReferenceResults gives you both the new and the altered results. Anyway, take it or leave it. But it does not help to complain again and again.
only update references where we have a reason
I actually wonder how MAP-Lib can figure out a-priori which simulation results change given a singular change on a base component, say e.g. https://github.com/modelica/ModelicaStandardLibrary/commit/e5f0e7eb5050456a8040d5a847adaab939cf866b. In my opinion you can only find out if you run a full regression. And in that case you get the altered results (as increment to the existing results). If a model is not affected, the CSV stays byte-identical and git can handle it.
I don't see opening issues to keep track of what still needs to be done before a release as complaining.
Hm, I have to admit I am even more confused. Are you asking for the (as previously done) manual job of running the simulation of a tagged release in Dymola and providing them (as described in https://github.com/modelica/ModelicaStandardLibrary/wiki/Reference-results) or are you asking for the commit-triggered increments of simulation results provided automatically by a CI? For the former you have my answer, for the latter MAP-Lib is not yet there.
We already have regression runs that run Dymola, comparing new results against the expected results from 4.0.0. We also have a collection of reported issues where these runs generate different results.
The next step is that library officers make a decision for each case where the results differ. Either they decide that the new results are expected because of changes in the library, and the reference results can then be updated (for the specific model). Alternatively, the difference is unexpected, and something has to be fixed in the library before release.
Just updating all references misses the important step of verifying that the new results are correct. Even if one does verify that, updating references that were within tolerances for the regression run, risks that the results drift away over time.
Of course, doing this step for each commit or each PR during development in a CI fashion is preferable, but as you say, we aren't there yet. I believe this is the main thing @casella wants us to focus on once 4.1.0 is ready.
Edit: @GallLeo and colleagues have already volunteered to do the actual update of the individual references once library officers indicate this is the way forward for specific tests. I believe there is already a draft PR with that work ongoing.
Anyway, take it or leave it
Better leave it. After spending some more time on https://github.com/modelica/ModelicaStandardLibrary/issues/4343 I only noticed now that some of the simulation result is no longer reproducable. Not sure what I did then (beside the too much manual patching of unusable MSL 4.1.0-beta.1). In order not to use it I removed that fork for now. Sorry for the inconvenience.
In #4081 variables were removed from the comparison signals. Hence, the reference results need to be updated.