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

Add mapping tester reference results #212

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/aste_ci_mac.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
- name: Setup system
run: |
brew update
brew install cmake eigen boost openmpi pkg-config ninja gnu-time vtk
brew install cmake eigen boost openmpi pkg-config ninja gnu-time vtk metis
- name: Install preCICE
run: |
git clone https://github.com/precice/precice.git
Expand Down
10 changes: 5 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required (VERSION 3.16.3)
cmake_minimum_required (VERSION 3.22)

project(ASTE)

Expand Down Expand Up @@ -103,9 +103,9 @@ else ()
)
endif()

file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/src/precice-aste-partition DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/src/precice-aste-join DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/src/precice-aste-evaluate DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
configure_file(src/precice-aste-partition . COPYONLY)
configure_file(src/precice-aste-join . COPYONLY)
configure_file(src/precice-aste-evaluate . COPYONLY)

include(GNUInstallDirs)
install(TARGETS precice-aste-run DESTINATION ${CMAKE_INSTALL_BINDIR})
Expand Down Expand Up @@ -134,7 +134,7 @@ foreach(example IN LISTS _examples)
set_tests_properties(aste.example.${example} aste.example.${example}.setup
PROPERTIES
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/examples/${example}"
ENVIRONMENT "PATH=$ENV{PATH}:${CMAKE_BINARY_DIR}"
ENVIRONMENT_MODIFICATION "PATH=path_list_append:${CMAKE_BINARY_DIR}"
LABELS example
RUN_SERIAL ON)
endforeach()
4 changes: 4 additions & 0 deletions examples/mapping_tester/reference-statistics.csv
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
2 changes: 2 additions & 0 deletions examples/mapping_tester/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,5 @@ cd "${TEST_LOCATION}"

# Gather the generated statistics
python3 "${MAPPING_TESTER}"/gatherstats.py --outdir "${TEST_CASE_LOCATION}" --file test-statistics.csv

python3 "${MAPPING_TESTER}"/compare.py reference-statistics.csv test-statistics.csv
3 changes: 3 additions & 0 deletions examples/mapping_tester/setup-test.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
},
"np": {
"kind": "nearest-projection"
},
"nn": {
"kind": "nearest-neighbor"
}
}
},
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
jinja2
numpy
polars
Copy link
Member

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.

Copy link
Member Author

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

Copy link
Member

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?

Copy link
Member Author

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.

scipy
sympy>=1.9
3 changes: 2 additions & 1 deletion src/precice-aste-evaluate
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,8 @@ class Calculator:
np.nanmax(abs_diff),
np.nanmax(difference),
)
p99, p95, p90, median = np.percentile(abs_diff, [99, 95, 90, 50])
p99, p95, p90 = np.percentile(abs_diff, [99, 95, 90])
median = np.median(abs_diff)
relative = np.sqrt(np.nansum(np.square(abs_diff)) / abs_diff.size)
decorator = 15 * "*"
spaces = 5 * " "
Expand Down
60 changes: 60 additions & 0 deletions tools/mapping-tester/compare.py
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
Copy link
Member

@davidscn davidscn Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be a problem with polars being called via ctest: in my venv it works locally (using run.sh), but ctest fails for me in the same bash session.

Copy link
Member Author

@fsimonis fsimonis Oct 28, 2024

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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 os.subprocess, maybe that's the simple answer.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+ python3 /Users/runner/work/aste/aste/examples/mapping_tester/../../tools/mapping-tester//compare.py reference-statistics.csv test-statistics.csv
The test results differ in both files
DataFrames are different (value mismatch for column 'median(abs)')
[left]:  [0.00565710724966223, 0.0004046891096261551]
[right]: [0.00567422622697189, 0.00040468910964214233]

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.

https://docs.pola.rs/api/python/stable/reference/api/polars.testing.assert_frame_equal.html#polars.testing.assert_frame_equal

Copy link
Member Author

@fsimonis fsimonis Nov 4, 2024

Choose a reason for hiding this comment

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

Maybe they changed the interpolation method? Still uses linear interpolation

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have the complete result table for both runs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be a matter of the column order or something?

Copy link
Member Author

Choose a reason for hiding this comment

The 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.
We never faced this issue before as we are now using separate cases, compared to
separate runs. Using topology-based partitioning solves the issue and leads to reproducible cases.

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)
2 changes: 1 addition & 1 deletion tools/mapping-tester/preparemeshes.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def preparePartMesh(meshdir, name, p, force=False):
"--mesh",
mainMesh,
"--algorithm",
"meshfree",
"topology",
"-o",
partMesh,
"--directory",
Expand Down
Loading