Add Time-Averaged Field Diagnostics #5285

n01r · 2024-09-18T01:23:52Z

This PR adds time-averaged field diagnostics to the WarpX output

code
docs
tests
example
meta-data $\Longrightarrow$ Follow-up PR
make compatible with adaptive time stepping $\Longrightarrow$ Follow-up PR

Currently, compiling for GPU creates a linking error. We would like to see the CI output to possibly find the issue.
Hence, we do not flag this as WIP right now.

This PR is based on work performed during the 2024 WarpX Refactoring Hackathon and was created together with @RevathiJambunathan. 🙂

Successfully merging this pull request may close #5165.

archermarx · 2024-09-18T13:05:56Z

Thanks for making this PR! One complication is that with #5176, adaptive timestepping will be possible. This means that the timestep length can change, so simply summing the multifabs and dividing by the period won't be enough for a proper time average. I don't think addressing this will be too hard, however.

n01r · 2024-09-18T16:33:15Z

Thanks, @archermarx! I wasn't aware of that yet. 🙂 We can do a follow-up and catch if dt_update_interval is set. In that case, we could create a cumulative sum weighted with the time step length and also add up the total time as we go. During the flush, we then divide by the total time.

archermarx · 2024-09-18T17:45:31Z

Thanks, @archermarx! I wasn't aware of that yet. 🙂 We can do a follow-up and catch if dt_update_interval is set. In that case, we could create a cumulative sum weighted with the time step length and also add up the total time as we go. During the flush, we then divide by the total time.

Yep, my thought exactly. The adaptive timestepping PR was just merged, too.

Source/Diagnostics/MultiDiagnostics.cpp

Source/Diagnostics/Diagnostics.cpp

Source/Diagnostics/FullDiagnostics.cpp

EZoni · 2024-09-19T00:16:53Z

I think #5296 should fix the clang-tidy errors that remain, which don't seem related to this PR.

ax3l · 2024-09-23T23:09:43Z

We can do a follow-up and catch if dt_update_interval is set
...
Yep, my thought exactly. The adaptive timestepping PR was just merged, too.

Thanks for the catch! Please add a runtime assert in this PR now.

n01r · 2024-09-24T19:13:41Z

Added commit that will cause the sim to abort when dt_update_interval is set.

The last thing for this PR that remains from our list is the CI tests.

@ax3l, should we also use the recently merged MultiFabRegister?

n01r · 2024-10-02T21:22:19Z

Just empirically, I tried this in our Laser-Ion Acceleration example. At step 1000, I compared between the electron density output from the in-situ generated averaged output and a post-processed output that I created from 25 instantaneous outputs. The relative difference is on the order of 1e-16. 🎉

Created with the following input script:
inputs_test_2d_laser_ion_acc.txt

n01r · 2024-10-02T21:53:34Z

And comparing instantaneous output with averaged output for, e.g. Ez shows the much clearer structure one can see with this diagnostic.

EZoni · 2024-10-16T19:53:20Z

@n01r @RevathiJambunathan
After resolving the merge conflicts, do you think the feature added in this PR is complete and ready to go, pending review?

n01r · 2024-10-16T20:16:21Z

@EZoni, the CI tests are still missing. I was thinking of adding these today since Perlmutter is under maintenance anyway.

n01r · 2024-10-18T21:59:57Z

Examples/Physics_applications/laser_ion/CMakeLists.txt

@@ -20,3 +30,14 @@ add_warpx_test(
 diags/diagInst/ # output
 OFF # dependency
 )
+
+# We do not have an intervals parser for PICMI yet which would accept more than a single integer for the output period


We should think about implementing an intervals parser for our PICMI diagnostics which can handle more than a single integer. It should be able to do the same as our app input: https://warpx.readthedocs.io/en/latest/usage/parameters.html#intervals-parser

Examples/Physics_applications/laser_ion/CMakeLists.txt

- Implement warnings and errors for certain inputs concerning averaging periods

- added first documentation on time averaged diags - put more operations on summation multifabs into if-environments

- added time averaged diags to laser-ion acceleration example - fix first issues that came up when testing this

for more information, see https://pre-commit.ci

Currently, we do not yet support the newly implemented adaptive time-stepping mode of electrostatic solvers together with time-averaged diagnostics.

Even though the laser ion test is named as a dependency it is running this test again. That should ideally be avoided. It would be good to just run the analysis script as test.

for more information, see https://pre-commit.ci

n01r · 2024-10-18T23:32:05Z

@EZoni, feel free to review :)

For PICMI, the TimeAveragedDiag test is automatically disabled because we cannot simply define the necessary output intervals. We need to be able to define period=[100,69:100] like for the app input.

for more information, see https://pre-commit.ci

EZoni · 2024-10-19T00:59:11Z

Examples/Physics_applications/laser_ion/analysis_test_laser_ion.py

+def analyze_openpmd_regression(output_file):
+ # Run checksum regression test
+ if re.search("single_precision", output_file):
+ evaluate_checksum(
+ test_name=test_name,
+ output_file=output_file,
+ output_format="openpmd",
+ rtol=2e-6,
+ )
+ else:
+ evaluate_checksum(
+ test_name=test_name,
+ output_file=output_file,
+ output_format="openpmd",
+ )
+
+


We want to change the way we handle single-precision tests anyways (see #5205), so if this test is not a single-precision test, I think we can remove this function and simply call evaluate_checksum below, consistently with the other analysis scripts in our test suite (since #5297):

Suggested change

def analyze_openpmd_regression(output_file):

# Run checksum regression test

if re.search("single_precision", output_file):

evaluate_checksum(

test_name=test_name,

output_file=output_file,

output_format="openpmd",

rtol=2e-6,

)

else:

evaluate_checksum(

test_name=test_name,

output_file=output_file,

output_format="openpmd",

)

EZoni · 2024-10-19T01:06:41Z

Examples/Physics_applications/laser_ion/analysis_test_laser_ion.py

+ test_name = os.path.split(os.getcwd())[1]
+ inst_output_file = sys.argv[1]
+
+ # Regression checksum test
+ # NOTE: works only in the example directory due to relative path import
+ analyze_openpmd_regression(inst_output_file)
+
+ # TODO: implement intervals parser for PICMI that allows more complex output periods
+ if "picmi" not in test_name:
+ # Functionality test for TimeAveragedDiagnostics
+ compare_time_avg_with_instantaneous_diags(
+ dir_inst=sys.argv[1],
+ dir_avg="diags/diagTimeAvg/",
+ )


Given the comment above, I would rewrite it like this:

Suggested change

test_name = os.path.split(os.getcwd())[1]

inst_output_file = sys.argv[1]

# Regression checksum test

# NOTE: works only in the example directory due to relative path import

analyze_openpmd_regression(inst_output_file)

# TODO: implement intervals parser for PICMI that allows more complex output periods

if "picmi" not in test_name:

# Functionality test for TimeAveragedDiagnostics

compare_time_avg_with_instantaneous_diags(

dir_inst=sys.argv[1],

dir_avg="diags/diagTimeAvg/",

)

# NOTE: works only in the example directory due to relative path import

# compare checksums

evaluate_checksum(

test_name=os.path.split(os.getcwd())[1],

output_file=sys.argv[1],

)

# TODO: implement intervals parser for PICMI that allows more complex output periods

test_name = os.path.split(os.getcwd())[1]

if "picmi" not in test_name:

# Functionality test for TimeAveragedDiagnostics

compare_time_avg_with_instantaneous_diags(

dir_inst=sys.argv[1],

dir_avg="diags/diagTimeAvg/",

)

If possible, I would leave the evaluate_checksum call as in this suggestion (instead of passing, for example, test_name=test_name with a test_name variable defined before) because it matches the format of this "unit code block" across the test suite and will make it easier to implement some upcoming changes in a more automatic way.

n01r added hackathon Let's address this topic during the GPU hackathon component: diagnostics all types of outputs labels Sep 18, 2024

EZoni reviewed Sep 18, 2024

View reviewed changes

Source/Diagnostics/MultiDiagnostics.cpp Show resolved Hide resolved

EZoni reviewed Sep 18, 2024

View reviewed changes

Source/Diagnostics/Diagnostics.cpp Outdated Show resolved Hide resolved

EZoni reviewed Sep 18, 2024

View reviewed changes

Source/Diagnostics/FullDiagnostics.cpp Outdated Show resolved Hide resolved

EZoni mentioned this pull request Sep 19, 2024

Fix clang-tidy errors in development branch #5296

Merged

n01r force-pushed the time_averaged_fields branch from af628fc to 822034d Compare September 20, 2024 22:40

ax3l self-assigned this Sep 23, 2024

n01r force-pushed the time_averaged_fields branch from 0f8c5e0 to 063f151 Compare September 24, 2024 19:08

RemiLehe assigned EZoni and unassigned ax3l Oct 16, 2024

n01r commented Oct 18, 2024

View reviewed changes

Examples/Physics_applications/laser_ion/CMakeLists.txt Outdated Show resolved Hide resolved

RevathiJambunathan and others added 7 commits October 18, 2024 15:10

skeleton sprinkled around

8dbde7d

Add time step interval logic

2707593

adding diag type to constructor and moving

a1b21d2

Add summation of MultiFabs

0aee250

Set time averaging options in ParmParse

1fef795

Continue averaging period logic

17d7620

- Implement warnings and errors for certain inputs concerning averaging periods

Add docs and scope code in if envs

c4b1775

- added first documentation on time averaged diags - put more operations on summation multifabs into if-environments

n01r and others added 9 commits October 18, 2024 15:17

Implement clang-tidy fixes

d4cb9ae

Add time averaged diags to laser ion example

ac216cc

- added time averaged diags to laser-ion acceleration example - fix first issues that came up when testing this

Add more warnings and user info

66adf7f

Add PICMI functionality

1bb3c7b

[pre-commit.ci] auto fixes from pre-commit.com hooks

c5f2e73

for more information, see https://pre-commit.ci

Add and fix docs

07e9b0e

Add abort for adaptive time-stepping

ea09ac2

Currently, we do not yet support the newly implemented adaptive time-stepping mode of electrostatic solvers together with time-averaged diagnostics.

Update inputs_test_2d_laser_ion_acc for Ex

98065f4

Added automated test based on LPI example

171ce3c

Even though the laser ion test is named as a dependency it is running this test again. That should ideally be avoided. It would be good to just run the analysis script as test.

n01r force-pushed the time_averaged_fields branch from bf71c29 to 171ce3c Compare October 18, 2024 22:31

[pre-commit.ci] auto fixes from pre-commit.com hooks

13cd6e4

for more information, see https://pre-commit.ci

Unify CI test for regression and TimeAvgDiag

22bc9a9

For PICMI, the TimeAveragedDiag test is automatically disabled because we cannot simply define the necessary output intervals. We need to be able to define period=[100,69:100] like for the app input.

n01r force-pushed the time_averaged_fields branch from 5979cc4 to 22bc9a9 Compare October 18, 2024 23:55

[pre-commit.ci] auto fixes from pre-commit.com hooks

0a46605

for more information, see https://pre-commit.ci

EZoni reviewed Oct 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Time-Averaged Field Diagnostics #5285

Add Time-Averaged Field Diagnostics #5285

n01r commented Sep 18, 2024 •

edited

Loading

archermarx commented Sep 18, 2024

n01r commented Sep 18, 2024

archermarx commented Sep 18, 2024

EZoni commented Sep 19, 2024

ax3l commented Sep 23, 2024

n01r commented Sep 24, 2024

n01r commented Oct 2, 2024 •

edited

Loading

n01r commented Oct 2, 2024

EZoni commented Oct 16, 2024

n01r commented Oct 16, 2024

n01r Oct 18, 2024

n01r commented Oct 18, 2024

EZoni Oct 19, 2024

EZoni Oct 19, 2024 •

edited

Loading

Add Time-Averaged Field Diagnostics #5285

Are you sure you want to change the base?

Add Time-Averaged Field Diagnostics #5285

Conversation

n01r commented Sep 18, 2024 • edited Loading

archermarx commented Sep 18, 2024

n01r commented Sep 18, 2024

archermarx commented Sep 18, 2024

EZoni commented Sep 19, 2024

ax3l commented Sep 23, 2024

n01r commented Sep 24, 2024

n01r commented Oct 2, 2024 • edited Loading

n01r commented Oct 2, 2024

EZoni commented Oct 16, 2024

n01r commented Oct 16, 2024

n01r Oct 18, 2024

Choose a reason for hiding this comment

n01r commented Oct 18, 2024

EZoni Oct 19, 2024

Choose a reason for hiding this comment

EZoni Oct 19, 2024 • edited Loading

Choose a reason for hiding this comment

n01r commented Sep 18, 2024 •

edited

Loading

n01r commented Oct 2, 2024 •

edited

Loading

EZoni Oct 19, 2024 •

edited

Loading