Fix checkpoint with runtime components through the Python interface. #2332

atmyers · 2021-09-23T21:47:47Z

Specifically, this makes three changes:

It adds the particle component names to the Checkpoint files
If a Checkpoint file contains a runtime component that has not yet been added, it will be added on the fly.
If a runtime component has already been added, subsequent calls not not add it again.

ax3l · 2021-09-24T02:00:07Z

Let's rebase this one after #2335 / #2341 are merged, which add more restart safeguards

Source/Diagnostics/ParticleIO.cpp

EtherealMC-Bit · 2021-09-27T16:25:28Z

The script in the original issue now verifies that the runtime attributes were added correctly, so it looks like this fixes it!

ax3l · 2021-09-27T17:36:56Z

@atmyers ready for rebase :)

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

…ntime_checkpoint

ax3l · 2021-09-27T22:21:49Z

Source/Diagnostics/FlushFormats/FlushFormatCheckpoint.cpp

@@ -159,8 +159,35 @@ FlushFormatCheckpoint::CheckpointParticles (
    const amrex::Vector<ParticleDiag>& particle_diags) const
 {
    for (unsigned i = 0, n = particle_diags.size(); i < n; ++i) {


In this function and potentially after this look, we probably also need to treat the laser particles to write our their positions

cc @RemiLehe

Support for this is added in PR #2360 - I recommend we merge that PR first, then rebase this one.

Source/Diagnostics/ParticleIO.cpp

ax3l · 2021-09-27T22:24:27Z

The current implementation looks good to me. Can we design or repurpose an existing test to cover this permanently? :)
cc @MKieburtz

EtherealMC-Bit · 2021-09-28T00:12:52Z

I'm not sure about an existing test, although the script in #2295 is a nice and clean way of demonstrating that this works correctly.

ax3l · 2021-09-28T04:55:39Z

LGTM, it's short and small. let's add this one? :)

ax3l · 2021-10-04T23:22:04Z

@atmyers thank you for #2360 - do you like to rebase the PR here now? :)

atmyers · 2021-10-07T18:48:12Z

I have now added a CI test for this, based on the original script from modern electron.

atmyers · 2021-10-07T18:50:24Z

Python/pywarpx/picmi.py

@@ -1085,6 +1086,10 @@ def initialize_inputs(self):
        self.diagnostic.diag_type = 'Full'
        self.diagnostic.format = 'checkpoint'

+        if self.write_dir is not None or self.file_prefix is not None:


I needed to modify the Python interface a bit here so that Checkpoint files could be configured to use a custom file prefix, as needed by the regression testing suite. Maybe @dpgrote could weigh in on whether this is the right way to fix this?

I could also split this into a separate PR, if needed.

LGTM. I think you can even remove the if line here with the logic in the next two lines

Let's also add an inline comment to document the need for this modification, otherwise this convention looks a bit magical to future readers

lgtm-com · 2021-10-07T19:33:35Z

This pull request introduces 1 alert when merging 2a1bfff into fb2ef4c - view on LGTM.com

new alerts:

1 for Unused import

ax3l · 2021-10-08T17:29:35Z

Python/pywarpx/picmi.py

+        if self.write_dir is not None or self.file_prefix is not None:
+            write_dir = (self.write_dir or 'diags')
+            file_prefix = (self.file_prefix or self.name)
+            self.diagnostic.file_prefix = write_dir + '/' + file_prefix


Suggested change

if self.write_dir is not None or self.file_prefix is not None:

write_dir = (self.write_dir or 'diags')

file_prefix = (self.file_prefix or self.name)

self.diagnostic.file_prefix = write_dir + '/' + file_prefix

# use checkpoint naming scheme expected by WarpX regression test suite

write_dir = (self.write_dir or 'diags')

file_prefix = (self.file_prefix or self.name)

self.diagnostic.file_prefix = write_dir + '/' + file_prefix

It's not really specific to the test suite - the other diagnostic types FieldDiagnostic and ParticleDiagnostic already let you pass in a custom file prefix for whatever reason you want, as you can in the C++ interface. This just makes the Checkpoints behave the same way. This if block is basically just copied from there.

Yes, this looks good. My preference is to keep the if check - this is a subtle difference but with the if, diagnostic.file_prefix won't be set if write_dir and file_prefix are not set and so it will rely on its default value in the C++ (this avoids having a default value defined in two different places).

I just noticed this - it should be self.diagnostic.file_prefix = os.path.join(write_dir, file_prefix) to be proper Python.

Sounds good - I'll issue a follow-up PR to change the + '/' +, I think the modern electron folks are waiting on this one to be merged.

Source/Diagnostics/FlushFormats/FlushFormatCheckpoint.cpp

ax3l

Thanks a lot for this, that's great!
The patch & test looks generally good to me.

Comment on the Python side and just double-checking the extra SoA attributes for ionization/QED are there.

atmyers added 5 commits September 23, 2021 10:57

add component names to checkpoint

50a0086

read in particle components and add the missing ones on restart

1b857bd

do not add components if one by the name already exists

229baa5

sign error

f585478

Better warning messages

4479c72

atmyers added bug: affects latest release Bug also exists in latest release version component: diagnostics all types of outputs labels Sep 23, 2021

ax3l self-assigned this Sep 24, 2021

ax3l self-requested a review September 24, 2021 01:59

ax3l reviewed Sep 24, 2021

View reviewed changes

Source/Diagnostics/ParticleIO.cpp Show resolved Hide resolved

ax3l added the bug Something isn't working label Sep 24, 2021

ax3l added the component: checkpoint/restart Checkpointing & restarts label Sep 27, 2021

atmyers and others added 3 commits September 27, 2021 12:28

Update Source/Diagnostics/ParticleIO.cpp

4a51d47

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

merging with development

0392230

Merge branch 'runtime_checkpoint' of github.com:atmyers/WarpX into ru…

f7e0ad3

…ntime_checkpoint

ax3l reviewed Sep 27, 2021

View reviewed changes

Merge branch 'development' into runtime_checkpoint

c6acc58

atmyers mentioned this pull request Oct 7, 2021

Handle customRunCmd in restart tests so we can test restarting through the python inferface. ECP-WarpX/regression_testing#15

Merged

add CI test for restarting particles with runtime-added attributes

2a1bfff

atmyers commented Oct 7, 2021

View reviewed changes

remove unused module

f94ddf7

ax3l reviewed Oct 8, 2021

View reviewed changes

Source/Diagnostics/FlushFormats/FlushFormatCheckpoint.cpp Show resolved Hide resolved

ax3l approved these changes Oct 8, 2021

View reviewed changes

atmyers merged commit 0c73e9b into ECP-WarpX:development Oct 8, 2021

ax3l mentioned this pull request Jan 28, 2022

Incomplete sets of particles loaded in on restart #2785

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix checkpoint with runtime components through the Python interface. #2332

Fix checkpoint with runtime components through the Python interface. #2332

atmyers commented Sep 23, 2021 •

edited by ax3l

Loading

ax3l commented Sep 24, 2021 •

edited

Loading

EtherealMC-Bit commented Sep 27, 2021

ax3l commented Sep 27, 2021

ax3l Sep 27, 2021

atmyers Sep 30, 2021

ax3l commented Sep 27, 2021 •

edited

Loading

EtherealMC-Bit commented Sep 28, 2021

ax3l commented Sep 28, 2021

ax3l commented Oct 4, 2021 •

edited

Loading

atmyers commented Oct 7, 2021

atmyers Oct 7, 2021

ax3l Oct 8, 2021 •

edited

Loading

lgtm-com bot commented Oct 7, 2021

ax3l Oct 8, 2021 •

edited

Loading

atmyers Oct 8, 2021

dpgrote Oct 8, 2021

atmyers Oct 8, 2021

ax3l left a comment •

edited

Loading

Fix checkpoint with runtime components through the Python interface. #2332

Fix checkpoint with runtime components through the Python interface. #2332

Conversation

atmyers commented Sep 23, 2021 • edited by ax3l Loading

ax3l commented Sep 24, 2021 • edited Loading

EtherealMC-Bit commented Sep 27, 2021

ax3l commented Sep 27, 2021

ax3l Sep 27, 2021

Choose a reason for hiding this comment

atmyers Sep 30, 2021

Choose a reason for hiding this comment

ax3l commented Sep 27, 2021 • edited Loading

EtherealMC-Bit commented Sep 28, 2021

ax3l commented Sep 28, 2021

ax3l commented Oct 4, 2021 • edited Loading

atmyers commented Oct 7, 2021

atmyers Oct 7, 2021

Choose a reason for hiding this comment

ax3l Oct 8, 2021 • edited Loading

Choose a reason for hiding this comment

lgtm-com bot commented Oct 7, 2021

ax3l Oct 8, 2021 • edited Loading

Choose a reason for hiding this comment

atmyers Oct 8, 2021

Choose a reason for hiding this comment

dpgrote Oct 8, 2021

Choose a reason for hiding this comment

atmyers Oct 8, 2021

Choose a reason for hiding this comment

ax3l left a comment • edited Loading

Choose a reason for hiding this comment

atmyers commented Sep 23, 2021 •

edited by ax3l

Loading

ax3l commented Sep 24, 2021 •

edited

Loading

ax3l commented Sep 27, 2021 •

edited

Loading

ax3l commented Oct 4, 2021 •

edited

Loading

ax3l Oct 8, 2021 •

edited

Loading

ax3l Oct 8, 2021 •

edited

Loading

ax3l left a comment •

edited

Loading