CTest: split checksum analysis from test analysis, expose arguments

[EZoni]

Prototype of implementation to see if this can achieve goals such as:

1. run test and analysis locally (possibly without CTest) without worrying about checksums
2. avoid duplicate code for default regression analysis with custom parameters (e.g., tolerance, output format)
3. minimize work needed to implement checksum regression analysis for new tests

This PR replaces #5447, see #5447 (comment).

Old usage

1. Add this to the test analysis script:

import os
import sys
...
...
sys.path.insert(1, "../../../../warpx/Regression/Checksum/")
from checksumAPI import evaluate_checksum
...
...
# compare checksums	
evaluate_checksum(	
    test_name=os.path.split(os.getcwd())[1],	
    output_file=sys.argv[1],	
)	

2. Add this to the CMakeLists.txt file:

add_warpx_test(
    test_1d_laser_acceleration_fluid_boosted  # name
    1  # dims
    2  # nprocs
    inputs_test_1d_laser_acceleration_fluid_boosted  # inputs
    analysis_1d_fluid_boosted.py  # analysis
    diags/diag1000001  # output
    OFF  # dependency
)

New usage

1. Create a soft link to the default regression analysis script from the test directory (we already do this when there isn't a custom analysis script, see comment in the follow-up section below):

ln -s ../../analysis_default_regression.py analysis_default_regression.py

2. Add this to the CMakeLists.txt file:

add_warpx_test(
    test_1d_laser_acceleration_fluid_boosted  # name
    1  # dims
    2  # nprocs
    inputs_test_1d_laser_acceleration_fluid_boosted  # inputs
    "analysis_1d_fluid_boosted.py diags/diag1000001"  # analysis
    "analysis_default_regression.py --path diags/diag1000001"  # checksum
    OFF  # dependency
)

Notes

• The updated default regression analysis script has the following usage:

usage: analysis_default_regression.py [-h] [--path PATH] [--rtol RTOL] [--skip-fields] [--skip-particles]
options:
  -h, --help        show this help message and exit
  --path PATH       path to output file(s)
  --rtol RTOL       relative tolerance to compare checksums
  --skip-fields     skip fields when comparing checksums
  --skip-particles  skip particles when comparing checksums

• The checksum files that changed (as opposed to the ones that were added from scratch) changed because the corresponding analysis scripts were not performing any checksum analysis. The files had been added but they were not used by the test analysis, hence they were outdated.

To-do

• Add missing checksum files or update existing checksum files that were not used
• Update documentation

Follow-up

• Improve documentation even more:
  • do we need all user-facing features of checksumAPI or are some obsolete?
  • can we merge documentation for testing and checksums into one section?
• Check/fix custom tolerances (git grep "# checksum" Examples/ | grep "rtol")
• Add logic to reset tolerances based on environment variables (e.g., to run all tests in single-precision)
• Update custom test analysis scripts so that they do not need to take the output path as argument (would be difficult to maintain as a convention, though, as these scripts are up to each PR author)
• Try (again) to make it work with direct path rather than soft link (did not work when we tried in the past)

[dpgrote]

I'm thinking about ways to make this simpler. Instead of having a soft link each place where the default analysis script is used, can this check if CHECKSUM_FILE is equal to analysis_default_regression.py and put in the appropriate direct path? Perhaps use some keyword like "DEFAULT" instead of the script name?

[EZoni]

I guess that would be nice, but when Axel and I set up ctest in the first place we did not manage to make it work with the direct paths (there were several attempts and then we turned to soft links). Especially, I think Axel found issues with running ctest from within an IDE. We could rivisit once he is back, but I would not try this here.

[dpgrote]

Thinking further, instead of writing out the full command to do the benchmark check, could the arguments to the analysis script be added as arguments to this function? In most cases, default values could be used so most CMakeLists.txt files wouldn't need any extra arguments (especially with the automatic discovery I suggest above). Then the rtol and skip-fields and skip-particles could be added via variable arguments pairs, like this

rtol 1.e-5

[EZoni]

I think this was another design choice that we made with Axel initially.

We wanted to keep the number of arguments of add_warpx_test as little as possible, especially given that we cannot use keywords like in Python (e.g., add_warpx_test(..., rtol=1e-6, ...)).

Another issue I see is that we would be treating the arguments to the custom analysis script (which cannot be predicted in advance) differently than the arguments to the default analysis script. It might be confusing to see a rtol argument and not know for sure whether it refers to the custom analysis script or to the default one, given that the interface of the custom analysis script is up to the developer who added it.

If you feel strongly about this, I would recommend that we wait until Axel is back and discuss with him further.

[EZoni]

Another comment is that we wanted to display commands that developers could copy/paste to try things locally, whenever that is needed (e.g., for debugging purposes). This came out of discussions with @aeriforme, and I think it would be valuable in general, as opposed to passing arguments to add_warpx_test when they are actually arguments read/used by the analysis scripts.

[dpgrote]

I don't feel strongly about this, just brain storming ideas. I see that having the command to do the benchmark check written out is convenient for debugging.

BTW, you can imitate keyword arguments though it's a bit kludgy. You can use variable arguments and scan through them. If one has a value of the argument name, then take the next argument as the value.

[dpgrote]

Can automatic discovery be used here instead of relying on the arguments? The checksum script could determine whether the output is a plotfile or openpmd. Also, the checksum would presumably always be done on the last diagnostic file and this could be automatically found. Then the path here would only be the top level diag directory.

[EZoni]

Yes, automating some of this was something I was hoping to do as well. Would you have a suggestion on how to automatically distinguish between plotfile and openpmd output?

[dpgrote]

One way would be to check for the Header file which would signify a plotfile diagnostic.

[EZoni]

I pushed something to see if this works:

    # set args.format automatically
    try:
        yt.load(args.path)
    except Exception:
        try:
            OpenPMDTimeSeries(args.path)
        except Exception:
            print("Could not open the file as a plotfile or an openPMD time series")
        else:
            args.format = "openpmd"
    else:
        args.format = "plotfile"

[dpgrote]

This looks good.

[EZoni]

@dpgrote

Hi Dave, as you saw I updated the default analysis script to detect the output file format automatically as you suggested. Personally I would postpone further automation and/or interface changes to follow-up work, due to the comments following #5456 (comment). Please feel free to leave another review.

[EZoni]

@aeriforme

Thanks to @dpgrote's comments, we made this even easier to use, with automatic detection of the output file format, which does not have to be passed as command line argument anymore. See updated PR description for the latest usage.

[EZoni]

@roelof-groenewald @RemiLehe

I added you as reviewers to this PR as well, since this is now at a mature stage and could be approved and merged soon. Please feel free to have a look, especially at the proposed feature rather then the individual code changes, and provide feedback, if any.

[aeriforme]

I see that it's possible to turn off the analysis with the OFF keyword.
I think this information is missing in the documentation.

What would happen if one used OFF instead of the checksums script?
Can we raise an error in that case, so we can ensure that all tests are checksummed?

[EZoni]

I think the behavior is the same as for the analysis script (i.e., it's skipped).

I agree that it should be documented, I will update the documentation asap.

I think it's a good idea to throw an error if the checksum analysis is skipped. I just have to double check that this is fine for all existing tests, but I guess it is.

[EZoni]

@aeriforme

Regarding this,

What would happen if one used OFF instead of the checksums script?
Can we raise an error in that case, so we can ensure that all tests are checksummed?

I think we cannot throw an error when the checksum command is set to OFF because we use that feature specifically for tests that need a preparatory step, such as the LASY tests ("output" here is "checksum" in this PR):

WarpX/Examples/Tests/laser_injection_from_file/CMakeLists.txt

Lines 64 to 82 in 2cdcb77

	add_warpx_test(
	test_2d_laser_injection_from_lasy_file_prepare # name
	2 # dims
	1 # nprocs
	inputs_test_2d_laser_injection_from_lasy_file_prepare.py # inputs
	OFF # analysis
	OFF # output
	OFF # dependency
	)
	add_warpx_test(
	test_2d_laser_injection_from_lasy_file # name
	2 # dims
	1 # nprocs
	inputs_test_2d_laser_injection_from_lasy_file # inputs
	analysis_2d.py # analysis
	diags/diag1000251 # output
	test_2d_laser_injection_from_lasy_file_prepare # dependency
	)

We did not manage to merge the preparatory step and the actual test into one single test when we set up ctest initially, I can't remember why but we ended up with this solution after trying to fix the LASY tests for a while, so I would not change this now.

[EZoni]

@aeriforme

Regarding this,

I see that it's possible to turn off the analysis with the OFF keyword.
I think this information is missing in the documentation.

I updated the documentation in 874d362.

[EZoni]

After discussing with @aeriforme, we added the particle data to the checksums of two RZ scraping tests (not clear why the particle data had been skipped when those tests were added in the first place).

To be double checked in the future if the first two values will raise issues due to the small order of magnitude:

    "particle_momentum_x": 8.802233511708275e-20,
    "particle_momentum_y": 8.865573181381068e-20,