search

ComputationalRadiationPhysics / picongpu

Performance-Portable Particle-in-Cell Simulations for the Exascale Era :sparkles:
https://picongpu.readthedocs.io
Other
710 stars 218 forks source link

openPMD plugin: Add option for Append mode in writing #5224

Open franzpoeschel opened 1 week ago

franzpoeschel commented 1 week ago

https://openpmd-api.readthedocs.io/en/0.16.0/usage/workflow.html#access-modes The Append mode is more secure as it protects better against accidentally overwriting data, e.g. when restarting from a checkpoint. I'm hesitant to fully replace the Create mode with Append mode, since (1) overwriting could be intended and (2) there were bugs in the past that made it impossible to create new files in Append mode with ADIOS2 on some platforms (I think mostly a Windows issue).

TODO:

psychocoderHPC commented 1 week ago

IMO for PIConGPU the default should be that we create a new file. If you like to append we should perform this explicit.

franzpoeschel commented 1 week ago

Just to make this clear: The only difference between both modes is when a file already exists. The current setting will truncate the file, the Append setting will add new Iterations, while leaving the old data intact. If no file exists yet, both modes will create a new file.

I have no strong preference for either option, so if you prefer sticking with the current setting, then I'll leave this as it is now.

psychocoderHPC commented 4 days ago

IMO keeping the current behavior is better. If someone is restarting a simulation after a checkpoint lets say at step 1000 but the simulation before was running until 1500 and than crashed at 1700 before writing a new checkpoint at 2000. If the output contains particles and we append data to the existing data at step 1500 we will have twice as many particles in the output. Thi will mean the output is broken because the data makes no sense. IMO append should be controlled explicitly and only used where it is guaranteed to be save.

PrometheusPi commented 12 hours ago

@franzpoeschel and @psychocoderHPC what is the conclusion of your discussion and what is the status of this PR (is it reviewable)?

psychocoderHPC commented 7 hours ago

Let me move this into draft that it will not be merged. The feature to set the mode is very important for workflows we need to initialize bunches but append should not be the default.