-
-
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?
Changes from all commits
d60267b
e08b82b
91f154d
d8afe47
b2fe78c
b464343
10472e6
1a74dd1
21cee5d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
count,abs_min,abs_max,signed_min:,signed_max,median(abs),relative-l2,99th percentile(abs),95th percentile(abs),90th percentile(abs),mapping,constraint,mesh A,mesh B,ranks A,ranks B,globalTime,mapDataTime,initializeTime,computeMappingTime,peakMemA,peakMemB | ||
3458,0.0,0.30692922919422094,-0.30692922919422094,0.24061844202860838,0.04128682075571033,0.07155821185793043,0.19914987149239097,0.14571976813321605,0.12332753716546282,nn,consistent,coarse_mesh,fine_mesh,2,2,103240.0,1.0,86208.0,1094.0,158692.0,163056.0 | ||
3458,6.5503158452884236e-15,0.01582390267257916,-0.01582390267257916,0.015021340046257325,0.0004046891096324279,0.002542045190476898,0.011347271673657122,0.005927831519688847,0.003794146590595022,tps,consistent,coarse_mesh,fine_mesh,2,2,136490.0,360.0,118898.0,347.0,160356.0,175488.0 | ||
3458,0.0,0.1512984513201765,-0.1324666295812572,0.1512984513201765,0.005843079556053121,0.015876505509115044,0.06508900308766544,0.030208461726809042,0.019931800023270732,np,consistent,coarse_mesh,fine_mesh,2,2,71487.0,20.0,53866.0,4897.0,158064.0,163824.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,9 @@ | |
}, | ||
"np": { | ||
"kind": "nearest-projection" | ||
}, | ||
"nn": { | ||
"kind": "nearest-neighbor" | ||
} | ||
} | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
jinja2 | ||
numpy | ||
polars | ||
scipy | ||
sympy>=1.9 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
#!/usr/bin/env python3 | ||
|
||
import argparse | ||
from sys import exit | ||
|
||
import polars as pl | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There seems to be a problem with polars being called via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A problem is that we currently overwrite the PATH at configuration time. So any further modifications are not picked up. I just saw that CMake 3.22 (our new baseline) provide ENV modification support for exact this use-case. https://cmake.org/cmake/help/v3.22/prop_test/ENVIRONMENT_MODIFICATION.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am wondering why it worked or works for the precice-profiling script (also used in the mapping tester), whereas it fails for the compare script. The precice-profiliing script is called through the gathering script There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my venv, the tests pass now. The CI doesn't seem to be happy https://github.com/precice/aste/actions/runs/11552451140/job/32151545870?pr=212#step:10:447 I am not sure if the actual solution here would be more simple. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Isn't this a property from the mapping accuracy that should be unchanged? The polars comparision uses rtol of 1e-05 and an atol of 1e-08. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ASTE generates, exports, partitions to VTU with binary data and uses the VTK-internal calculator. The only moving targets are the VTK version and the numpy version, which we use to calculate the percentiles/median.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The assert checks col by col and the first column is the median. So it is possible that other columns don't match too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have the complete result table for both runs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could it be a matter of the column order or something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After a morning of debugging we realised that the partitioning wasn't deterministic, which lead to major differences in NP and even tiny run to run differences in RBF TPS. |
||
from polars.testing import assert_frame_equal | ||
|
||
parser = argparse.ArgumentParser() | ||
parser.add_argument("reference") | ||
parser.add_argument("current") | ||
args = parser.parse_args() | ||
|
||
df_a = pl.read_csv(args.current) | ||
df_reference = pl.read_csv(args.reference) | ||
|
||
ac = set(df_a.columns) | ||
rc = set(df_reference.columns) | ||
if ac != rc: | ||
print(f"Columns don't match!") | ||
print(f"Missing: {' '.join(rc - ac)}") | ||
print(f"Unexpected: {' '.join(ac - rc)}") | ||
exit(1) | ||
|
||
# columns defining test case | ||
order = ["mesh A", "mesh B", "mapping", "constraint", "ranks A", "ranks B"] | ||
missing = set(order) - ac | ||
if missing: | ||
print(f"Columns are missing from test setup: {' '.join(missing)}") | ||
exit(1) | ||
|
||
# Ensure the case order is identical | ||
df_a = df_a.sort(order) | ||
df_reference = df_reference.sort(order) | ||
|
||
# Test if the setups are identical | ||
try: | ||
assert_frame_equal(df_a.select(order), df_reference.select(order)) | ||
except AssertionError as e: | ||
print("The test setup differs in both files") | ||
print(e) | ||
exit(1) | ||
|
||
# Time and memory columns if they are valid | ||
posCols = {c for c in ac for m in ["time", "mem"] if m in c.lower()} | ||
for pc in posCols: | ||
if not (df_a.select(pc).to_numpy() > 0.0).all(): | ||
print(f"Column {pc} contains values <= 0") | ||
exit(1) | ||
|
||
# Test data columns if they approximately the same | ||
try: | ||
assert_frame_equal(df_a.drop(posCols), df_reference.drop(posCols)) | ||
except AssertionError as e: | ||
print("The test results differ in both files") | ||
print(e) | ||
exit(1) | ||
|
||
print("Both files are the same") | ||
exit(0) |
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 needsprecice-profiling analyze
which requirespolars
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.