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

Regression test reference files generated with Mac M3 / ARM may cause trouble #3244

Closed
nkuehnel opened this issue Apr 30, 2024 · 4 comments
Closed

Comments

@nkuehnel
Copy link
Member

nkuehnel commented Apr 30, 2024

I had some trouble updating test reference files that were generated on my Mac with M3 chip when working on this PR (tests ran through locally but not on github):
#3224

A problem might be that the population equals method relies on raw bytes of gzipped files:

/**
* Compares two Populations by serializing them to XML with the current writer
* and comparing their XML form byte by byte.
* A bit like comparing checksums of files,
* but regression tests won't fail just because the serialization changes.
* Limitation: If one of the PopulationWriters throws an Exception,
* this will go unnoticed (this method will just return true or false,
* probably false, except if both Writers have written the exact same text
* until the Exception happens).
*/
public static boolean equalPopulation(final Population s1, final Population s2) {
try {
try( InputStream inputStream1 = openPopulationInputStream( s1 ) ; InputStream inputStream2 = openPopulationInputStream( s2 ) ){
return IOUtils.isEqual( inputStream1, inputStream2 );
}
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

Changing to the population comparison class
https://github.com/matsim-org/matsim-libs/blob/master/matsim/src/main/java/org/matsim/core/population/routes/PopulationComparison.java

and updating it to include pairwise plan elements comparison in
564cf47

worked.

In lack of a better explanation so far I'm leaving this issue as a FYI in case someone runs into the same issue.

If the diagnosis is correct, we might even think of removing the by-byte comparison of populations (which isn't that helpful in terms of identifying what is different anyways.. the events file comparator is much more helpful in that regard)

@michalmac
Copy link
Member

I remember @luchengqi7 ran into the same issue several times. This was related to comparing scores. The problem is that the FP works sometimes (*) a bit different on mac and intel CPUs.

(*) AFAIR this difference is not coming from simple FP arithmetic operators. It's more related to more complex calculations, e.g. calling the exp() function (which I think we do in scoring).

@michalmac
Copy link
Member

I think asserting the equality of plan scores with some (even very small) delta should be enough.

@michalmac
Copy link
Member

(*) AFAIR this difference is not coming from simple FP arithmetic operators. It's more related to more complex calculations, e.g. calling the exp() function (which I think we do in scoring).

This could depend on the specific JDK distribution and the math libs it uses. But I am not expert at all here.

@nkuehnel
Copy link
Member Author

nkuehnel commented May 2, 2024

I tried to replace the comparison in other tests as well
#3247

@nkuehnel nkuehnel closed this as completed May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants