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 repetitions as separate script #226

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

fsimonis
Copy link
Member

@fsimonis fsimonis commented Jan 28, 2025

Main changes of this PR

Based on #225
Replaces #195

This PR adds a separate python script repeat.py to the mapping tester to handle repetitions, which are useful for runtime measurements.

It repeats for each repetition:

  1. runall
  2. postprocessall
  3. gatherstat into file-n.csv

Then it merges all N file-n.csv into a single file.csv.
The column run indicates which repetition it was.

Author's checklist

  • I used the pre-commit hook and used pre-commit run --all to apply all available hooks.
  • I added a test to cover the proposed changes in our test suite.
  • I updated the documentation in docs/README.md.
  • I added a changelog entry in ./changelog-entries/ (create if necessary).
  • I updated potential breaking changes in the tutorial precice/tutorials/aste-turbine.

Copy link
Member

@davidscn davidscn left a comment

Choose a reason for hiding this comment

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

There is currently no option to recover details about individual runs, once all repetitions are completed, right? Or do we store the json file?

Copy link
Member

Choose a reason for hiding this comment

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

I think with this we should also provide a script to process the data statistically across various runs, no?

@fsimonis
Copy link
Member Author

fsimonis commented Jan 28, 2025

The script currently doesn't clean up intermediates. Running it for 5 repetitions results in:

test-statistics-0.csv
test-statistics-1.csv
test-statistics-2.csv
test-statistics-3.csv
test-statistics-4.csv
test-statistics.csv

The various profiling.json files are not stored.
Isn't it easier to integrate the interesting events directly into gatherstats?

Copy link
Member

@davidscn davidscn left a comment

Choose a reason for hiding this comment

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

Ah ok. I think then this essentially a python version of what I once implemented in bash. The nice thing about the bash script was actually that it served a central steering component around all the scripts in the mapping tester. However, you can still add the repetition script provided here, as one can still use the bash version as a central instance if deemed necessary

@fsimonis
Copy link
Member Author

fsimonis commented Jan 28, 2025

Added a simple aggregation script for mean median and variance with support for dropping min and max.
I integrated it into the runtime tests, even though the column values will not be checked.

Will add this as a separate PR. Adding backends for both polars and pandas makes this a bit painful.

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

Successfully merging this pull request may close these issues.

2 participants