-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add mapping tester reference results #212
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great step for more robust testing. Currently, tests are failing.
ac9c3ca
to
91f154d
Compare
@@ -1,4 +1,5 @@ | |||
jinja2 | |||
numpy | |||
polars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would require a documentation update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is primarily to get the CI working. I'll revert later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it will remain a requirement to execute the tests, right? In the past, we also had tests for the mapping-tester here, which has optional dependencies. Then, however, users install the software, run ctest
and see that tests are failing.
How does this behave here then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, the mapping-tester
-test needs precice-profiling analyze
which requires polars
anyway. So the dependency is already there.
If we merge #210, then the timings and thus the dependency on polars becomes optional.
In that case, it may be an idea to implement the compare without polars.
On the flip side, this then becomes a bad test, as it tests two different things depending on the environment. Maybe a warning in CMake is enough here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added polars to the documentation.
I rewrote the compare file to be actually helpful now. |
507e94d
to
8898237
Compare
The results are still strange. When running With clang (both) release
With gcc (preCICE) release I get
With gcc (both) release I get
In the CI, we get Clang debug
Clang release
GCC Debug
GCC Release
So, there seems to be enough variation in the nearest-projection to result in failing tests. The big question is why are these projection results different. Turned out that the Metis partitioning is also non-deterministic. |
5080e84
to
a70c9cd
Compare
a70c9cd
to
48df5db
Compare
The NP mappings are indeed different between versions 1.86 and 1.87 of boost. |
The solution should be unique, or am I missing something? This sounds like a bug either in preCICE or boost geometry. The release notes of boost 1.87 do not mention anything which could sound related. |
So, the mappings map to different elements, but the effective difference in distance is tiny. I think this happens when a point is located on the edge between two triangles. Here I print out the polation elements and weights and then compute the projection point for the old and new boost. Given that our input functions are spacially dependent, this shouldn't lead to such a big difference in the output. Or could the different points impact the result this extremely in the output of ASTE?
|
Main changes of this PR
This PR adds a mapping tester compare script and reference results.
The script checks:
Closes #208
Author's checklist
pre-commit
hook and usedpre-commit run --all
to apply all available hooks.docs/README.md
../changelog-entries/
(create if necessary).precice/tutorials/aste-turbine
.