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

CTest follow-up: expand range of tolerances used for checksum benchmarks #5256

Open
EZoni opened this issue Sep 12, 2024 · 2 comments
Open
Assignees
Labels
component: tests Tests and CI

Comments

@EZoni
Copy link
Member

EZoni commented Sep 12, 2024

Expand on what is described in #5205, expose more tolerances:

  • single-precision tolerance
  • double-precision loose tolerance (for hardware portability)
  • double-precision strict tolerance (for same-machine benchmarks)
@EZoni
Copy link
Member Author

EZoni commented Sep 18, 2024

Copied from #5205 (comment) because it is relevant here as well:

To generalize the analysis scripts, we could change the APIs of the tolerance argument to the checksum API, and pass a dictionary (double and single as keys) for the tolerances. Or have two arguments, one double tolerance one float (with a dictionary, we keep it open to have other types later, but I do not think this is timely). Inside the checksum API, we could do a test for WarpX config used or type of a data file entry read to determine if something was a SP or DP run.

@EZoni
Copy link
Member Author

EZoni commented Sep 18, 2024

@ax3l

I guess I would go for multiple arguments, say

  • rtol_single_portable (default: ...)
  • rtol_double_portable (default: ...)
  • rtol_double_machine_precision (default ...)

because I don't see a good way to handle defaults if we pass a dictionary.

For example, with multiple arguments, we could do something like

checksumAPI.evaluate_checksum(testname, filename, rtol_double_portable=1e-3)

whenever a test needs to pass a larger "double-portable" tolerance for whatever reason, without having to worry about the defaults for rtol_single_portable and rtol_double_machine_precision, if they don't need adjustments.

On the other end, how would we do this if we have to pass a dictionary? Something like

checksumAPI.evaluate_checksum(testname, filename, tol_dict={"single_portable": None, "double_portable": 1e-3, "double_machine_precision": None})

and then having, inside checksumAPI, the logic that uses defaults if None is passed, maybe?

I feel like the first option could be easier to use.

@EZoni EZoni mentioned this issue Sep 19, 2024
12 tasks
ax3l pushed a commit that referenced this issue Oct 4, 2024
Fix #5206 and prepare for #5256:
- [x] Add missing checksums
  - [x] `test_1d_background_mcc_picmi`
  - [x] `test_2d_background_mcc_picmi`
  - [x] `test_3d_particle_absorption`
  - [x] `test_2d_dirichlet_bc`
  - [x] `test_2d_dirichlet_bc_picmi`  
  - [x] `test_2d_field_probe` 
  - [x] `test_3d_embedded_boundary_picmi`
  - [x] `test_2d_particle_reflection_picmi` 
  - [x] `test_rz_laser_acceleration_opmd` 
  - [x] `test_2d_runtime_components_picmi` 
- [x] Clean up calls to `evaluate_checksum`

Calls to `evaluate_checksum` are now uniform across the analysis
scripts, making it easier to automatize further changes. Hopefully the
uniformity will be preserved by copy/pasting the necessary code:
- ```py
  import os
  import sys
  ```
- ```py
  sys.path.insert(1, "../../../../warpx/Regression/Checksum/")
  from checksumAPI import evaluate_checksum
  ```
- ```py
  # compare checksums
  evaluate_checksum(
      test_name=os.path.split(os.getcwd())[1],
      output_file=sys.argv[1],
  )
  ```
with this last one possibly modified by adding other arguments, e.g.,
`output_format`, `rtol`, `do_particles`, etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tests Tests and CI
Projects
None yet
Development

No branches or pull requests

1 participant