openpmd.py plugin for PICMI #5353

mafshari64 · 2025-04-28T11:11:26Z

BrianMarre · 2025-04-28T11:41:54Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

+        if self.data_preparation_strategy is not None and self.data_preparation_strategy not in [
+            "doubleBuffer",
+            "adios",
+            "mappedMemory",
+            "hdf5",
+        ]:


we might want to use Literals and enforce them via the type system in the constructor

BrianMarre · 2025-04-28T11:43:14Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

+        source: Optional[Source] = None,
+        range: Optional[str] = None,
+        file: Optional[str] = None,
+        ext: Optional[str] = None,


the ext parameter should defined as a literal not a str

BrianMarre · 2025-04-28T11:54:27Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

+    def __init__(
+        self,
+        period: TimeStepSpec,
+        source: Optional[Source] = None,
+        range: Optional[str] = None,
+        file: Optional[str] = None,
+        ext: Optional[str] = None,
+        infix: Optional[str] = None,
+        json: Optional[Union[str, Dict]] = None,
+        json_restart: Optional[Union[str, Dict]] = None,
+        data_preparation_strategy: Optional[str] = None,
+        toml: Optional[str] = None,
+        particle_io_chunk_size: Optional[int] = None,
+        file_access: Optional[str] = None,


the default value slisted in the documentation do not match the actual constructor defaults

follwoing here we have

--openPMD.source | Select data sources and filters to dump. Default is species_all,fields_all,
for this reason self.source = source if source is not None else Source(["species_all", "fields_all"]). is it still wrong?

that's not what I meant, for example
the default for ext is bp following the documentation, but the constructor default argument is None it should be bp instead.
Please match the default arguments in the documentation to the defaults specified by the constructor.

And more general, most of the settings are currently PIConGPU specific. That is fine for now but we have to figure out a way to abstract that in the future

BrianMarre · 2025-04-28T12:01:24Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

+    """
+    Specifies the parameters for the openPMD plugin in PIConGPU simulations.


the PICMI implementation should not reference PIConGPU

BrianMarre · 2025-04-28T12:03:19Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

+    source: Source, optional
+        Data sources and filters to dump (e.g., Source([ChargeDensity(filter="filterX"), "species_all"])).
+        Default: Source(["species_all", "fields_all"]).


I am not a fan of allowing "str" specification of sources, that jsut adds another way to get the same results and I would prefer just one good way to specify stuff.

BrianMarre · 2025-04-28T12:11:27Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/source.py

+class Source:
+    """
+    Consolidates data sources for the openPMD plugin in PIConGPU.
+
+    This class aggregates individual data sources (e.g., ChargeDensity) or predefined
+    keywords (species_all, fields_all) to define the --openPMD.source parameter:
+    https://picongpu.readthedocs.io/en/latest/usage/plugins/openPMD.html
+
+    Parameters
+    ----------
+    sources: List[Union[str, <DataSourceClasses>]]
+        List of data sources, either as strings (e.g., "species_all", "fields_all") or
+        as data source objects (e.g., ChargeDensity, Density).
+    """


@mafshari64 could you please comment on why you consider the aggregator functionality class necessary? I currently don' see why it is needed, it looks like the entire functionality could be moved to the openPMD plugin.
Instead I would use this class as an interface for all source implementations

BrianMarre · 2025-04-28T12:16:35Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/bound_electron_density.py

+@typeguard.typechecked
+class BoundElectronDensity:


this class should inherit from a common interface of all Sources for the openPMD plugin

BrianMarre · 2025-04-28T12:21:05Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/bound_electron_density.py

+            The dataset name with optional filter (e.g., "bound_electron_density" or
+            "bound_electron_density:filterX").
+        """
+        if self.filter:
+            return f"bound_electron_density:{self.filter}"
+        return "bound_electron_density"


the conversion to strings should not happen on the PICMI side, that string is already PIConGPU specific

BrianMarre · 2025-04-28T12:24:33Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/charge_density.py

+    def get_source_string(self) -> str:
+        """
+        Return the source string for use in --openPMD.source.
+
+        Returns
+        -------
+        str
+            The dataset name with optional filter (e.g., "charge_density" or "charge_density:filterX").
+        """
+        if self.filter:
+            return f"charge_density:{self.filter}"
+        return "charge_density"


same problem here, the source string should happen at the earliest in PyPIConGPU, as the generated string is PIConGPU specific.
also the generated string is wrong, it should be source_all

BrianMarre · 2025-04-28T12:28:10Z

lib/python/picongpu/pypicongpu/output/openpmd.py

+                str,
+                ChargeDensity,
+                BoundElectronDensity,
+                Counter,
+                Density,
+                Energy,
+                EnergyDensity,
+                EnergyDensityCutoff,
+                LarmorPower,
+                MacroCounter,
+                MidCurrentDensityComponent,
+                Momentum,
+                MomentumDensity,
+                WeightedVelocity,
+            ]
+        ]
+    )
+
+    def __init__(
+        self,
+        sources: typing.List[
+            typing.Union[
+                str,
+                ChargeDensity,
+                BoundElectronDensity,
+                Counter,
+                Density,
+                Energy,
+                EnergyDensity,
+                EnergyDensityCutoff,
+                LarmorPower,
+                MacroCounter,
+                MidCurrentDensityComponent,
+                Momentum,
+                MomentumDensity,
+                WeightedVelocity,
+            ]


I am not a fan of the many repeats of the all the different source,please switch to using interfaces and testing with isinstance for the interface

BrianMarre · 2025-04-28T12:39:52Z

fundamentally and in summary, the current PR lacks an common interface for all the current sources of the openPMD plugin and this interface is required.

BrianMarre

general the current version is still missing the configuration of species contributing to the derived field sources.

BrianMarre · 2025-04-29T08:24:24Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/charge_density.py

+    def __init__(self, filter: typing.Optional[str] = None):
+        self.filter = filter
+        self.check()


default filter should be all, as this is the PIConGPU standard. And since we will stick to str for now, I would like to avoid using Optional here as it only complicates the implementation.

In addition, this class is missing the option to configure the species contributing to this source, please add a list of species that should contribute to this source.

BrianMarre · 2025-04-29T08:30:12Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/bound_electron_density.py

+    def __init__(self, filter: typing.Optional[str] = None):
+        self.filter = filter
+        self.check()


same problem as charge density source, the user must be able to specify the species that will contribute to the derived field source

BrianMarre

looks generally good, but documentation strings should use doxygen annotations

BrianMarre · 2025-04-30T08:06:57Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

+    """
+    Specifies parameters for openPMD diagnostic output in particle-in-cell simulations.
+
+    This diagnostic outputs simulation data (fields and particles) to disk using the openPMD
+    standard, with configurable periods, data sources, and backend settings.
+
+    Attention: **period is mandatory.**
+
+    Parameters
+    ----------
+    period: TimeStepSpec
+        Specifies the time steps for data output.
+        Unit: simulation time steps. Required.
+
+    source: List[SourceBase], optional
+        List of data source objects to dump (e.g., [ChargeDensity(filter="all")]).
+        Default: None (no sources specified).
+
+    range: str, optional
+        Contiguous range of cells per dimension to dump (e.g., ":,:,:").
+        Format: comma-separated ranges like "begin:end,begin:end,begin:end".
+        Default: ":,::,:" (all cells).
+
+    file: str, optional
+        Relative or absolute file prefix for openPMD output files.
+        If relative, files are stored under the simulation output directory.
+        Default: None (backend-dependent default).
+
+    ext: Literal["bp", "h5", "sst"], optional
+        File extension controlling the openPMD backend.
+        Options: "bp" (ADIOS2), "h5" (HDF5), "sst" (ADIOS2/SST for streaming).
+        Default: "bp" (ADIOS2 backend).
+
+    infix: str, optional
+        Filename infix for iteration layout (e.g., "_%06T").
+        Use "NULL" for group-based layout. Required as "NULL" if ext="sst".
+        Default: "NULL" (group-based layout).
+
+    json: Union[str, Dict], optional
+        Backend-specific parameters for writing, as a JSON string, dictionary, or filename
+        (filename must be prepended with "@").
+        Default: {} (empty dictionary).
+
+    json_restart: Union[str, Dict], optional
+        Backend-specific parameters for restarting, as a JSON string, dictionary, or filename
+        (filename must be prepended with "@").
+        Default: {} (empty dictionary).
+
+    data_preparation_strategy: Literal["doubleBuffer", "adios", "mappedMemory", "hdf5"], optional
+        Strategy for particle data preparation.
+        Options: "doubleBuffer" or "adios" (ADIOS2-based), "mappedMemory" or "hdf5" (HDF5-based).
+        Default: None (backend-dependent).
+
+    toml: str, optional
+        Path to a TOML file for openPMD configuration.
+        Default: None.
+
+    particle_io_chunk_size: int, optional
+        Size of particle data chunks for writing (in MiB).
+        Reduces host memory footprint for certain backends.
+        Default: None (backend-dependent).
+
+    file_access: Literal["create", "append"], optional
+        File access mode for writing.
+        Options: "create" (new files), "append" (for checkpoint-restart workflows).
+        Default: "create" (new files).
+    """


Suggested change

"""

Specifies parameters for openPMD diagnostic output in particle-in-cell simulations.

This diagnostic outputs simulation data (fields and particles) to disk using the openPMD

standard, with configurable periods, data sources, and backend settings.

Attention: **period is mandatory.**

Parameters

----------

period: TimeStepSpec

Specifies the time steps for data output.

Unit: simulation time steps. Required.

source: List[SourceBase], optional

List of data source objects to dump (e.g., [ChargeDensity(filter="all")]).

Default: None (no sources specified).

range: str, optional

Contiguous range of cells per dimension to dump (e.g., ":,:,:").

Format: comma-separated ranges like "begin:end,begin:end,begin:end".

Default: ":,::,:" (all cells).

file: str, optional

Relative or absolute file prefix for openPMD output files.

If relative, files are stored under the simulation output directory.

Default: None (backend-dependent default).

ext: Literal["bp", "h5", "sst"], optional

File extension controlling the openPMD backend.

Options: "bp" (ADIOS2), "h5" (HDF5), "sst" (ADIOS2/SST for streaming).

Default: "bp" (ADIOS2 backend).

infix: str, optional

Filename infix for iteration layout (e.g., "_%06T").

Use "NULL" for group-based layout. Required as "NULL" if ext="sst".

Default: "NULL" (group-based layout).

json: Union[str, Dict], optional

Backend-specific parameters for writing, as a JSON string, dictionary, or filename

(filename must be prepended with "@").

Default: {} (empty dictionary).

json_restart: Union[str, Dict], optional

Backend-specific parameters for restarting, as a JSON string, dictionary, or filename

(filename must be prepended with "@").

Default: {} (empty dictionary).

data_preparation_strategy: Literal["doubleBuffer", "adios", "mappedMemory", "hdf5"], optional

Strategy for particle data preparation.

Options: "doubleBuffer" or "adios" (ADIOS2-based), "mappedMemory" or "hdf5" (HDF5-based).

Default: None (backend-dependent).

toml: str, optional

Path to a TOML file for openPMD configuration.

Default: None.

particle_io_chunk_size: int, optional

Size of particle data chunks for writing (in MiB).

Reduces host memory footprint for certain backends.

Default: None (backend-dependent).

file_access: Literal["create", "append"], optional

File access mode for writing.

Options: "create" (new files), "append" (for checkpoint-restart workflows).

Default: "create" (new files).

"""

"""

openPMD diagnostic output

This diagnostic writes simulation data (base fields , derived fields and/or particles) to disk using the openPMD

standard, with configurable periods, data sources and backend settings.

@param period specification of the the time steps for data output, outputs will always be written at the end of a PIC time step.

@param source list of data source objects to include in the dump (e.g., [ChargeDensity(filter="all")]),

Setting this to None will cause an empty dump

@param range contiguous range of cells to dump the base- and derived field for

specification as comma-separated "begin:end" range for each dimension, i.e. "begin:end,begin:end,begin:end"

Notes: Values will be clipped to the simulation box. Begin and/or end may be omitted to indicate the limit of the simulation box in the dimension. The Default value ":,::,:" indicates that fields should be dumped for all cells.

@param file relative or absolute file path prefix for openPMD output files. Relative path are interpreted as relative to the simulation output directory, the default value None indicates the backend's default.

@param ext file extension controlling the openPMD backend, options are "bp" (default backend ADIOS2), "bp4" (bp4 backend ADIOS2), "bp5" (bp5 backend ADIOS2), "h5" (HDF5), "sst" (ADIOS2/SST for streaming).

@param infix filename infix for the iteration layout (e.g., "_%06T"), use "NULL" for the group-based layout, ext="sst" requires infix="NULL".

@param json openPMD backend configuration as a JSON string, dictionary, or filename (filename must be prepended with "@").

@param json_restart: backend-specific parameters for restarting, as a JSON string, dictionary, or filename (filenames must be prepended with "@").

@param data_preparation_strategy: strategy for particle data preparation, options: "doubleBuffer" or "adios" (ADIOS2-based), "mappedMemory" or "hdf5" (HDF5-based), the default value None indicates the backend default

@param toml path to a TOML file for openPMD configuration. Replaces the JSON or keyword configuration.

@param particle_io_chunk_size size of particle data chunks used in writing (in MiB), reduces host memory footprint for certain backends, default "None" indicates the backend default.

@param file_access file access mode for writing, options: "create" (new files), "append" (for checkpoint-restart workflows).

"""

please use the doxygen annotations for the documentation.

thanks for review,
a short question: in other picmi/diagnositcs files I used standard Python docstring format eg lib/python/picongpu/picmi/diagnostics/macro_particle_count.py and not oxygen annotations. just for this file
lib/python/picongpu/picmi/diagnostics/openpmd.py for documentation or do for all?

PrometheusPi

just a first review - no test yet

PrometheusPi · 2025-05-06T08:54:59Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

        Contiguous range of cells per dimension to dump (e.g., ":,:,:").
        Format: comma-separated ranges like "begin:end,begin:end,begin:end".
-        Default: None (all cells).
+        Default: ":,::,:" (all cells).


I think this is a typo, right?

Suggested change

Default: ":,::,:" (all cells).

Default: ":,:,:" (all cells).

PrometheusPi · 2025-05-06T08:55:56Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

        File extension controlling the openPMD backend.
        Options: "bp" (ADIOS2), "h5" (HDF5), "sst" (ADIOS2/SST for streaming).
-        Default: None (backend-dependent).
+        Default: "bp" (ADIOS2 backend).


This is not ensured, as we might compile with hdf5 only

Do you really want to hard code your selection below here?

I just mentioned bp as the default as it is in documentation not hardcoded. users can specify what they want.

PrometheusPi · 2025-05-06T08:57:26Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/auto.py

@@ -0,0 +1,59 @@
+"""
+This file is part of PIConGPU.
+Copyright 2021-2024 PIConGPU contributors


Suggested change

Copyright 2021-2024 PIConGPU contributors

Copyright 2021-2025 PIConGPU contributors

Why does it start in 2021? Was it copied from somewhere?

sorry I forgot to change it. I will do

PrometheusPi · 2025-05-06T08:58:54Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/auto.py

+    Represents a default data source for openPMD output in particle-in-cell simulations.
+
+    This class specifies a backend-specific default source for dumping simulation data
+    using the openPMD standard. For example, in some backends, it may dump all particle


Why only in some backends. As I understand it, backends are e.g. ADIOS or HDF5, but that doesn't determine what you write out.

PrometheusPi · 2025-05-06T08:59:34Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/auto.py

+    ----------
+    filter: str, optional
+        Name of a filter to select data contributing to the source.
+        Default: None (backend-dependent).


Why is the filter backend dependent. Our filters are defined in PIConGPU.

PrometheusPi · 2025-05-06T09:00:00Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/bound_electron_density.py

 """
 This file is part of PIConGPU.
-Copyright 2021-2025 PIConGPU contributors
+Copyright 2021-2024 PIConGPU contributors


Suggested change

Copyright 2021-2024 PIConGPU contributors

Copyright 2021-2025 PIConGPU contributors

Why does it start in 2021? Was it copied from some file?

PrometheusPi · 2025-05-06T09:02:34Z

lib/python/picongpu/picmi/diagnostics/openpmd_sources/charge_density.py

 """
 This file is part of PIConGPU.
-Copyright 2021-2025 PIConGPU contributors
+Copyright 2021-2024 PIConGPU contributors


same as above

PrometheusPi · 2025-05-06T09:03:44Z

lib/python/picongpu/pypicongpu/output/openpmd_source/bound_electron_density.py

@@ -0,0 +1,34 @@
+"""
+This file is part of PIConGPU.
+Copyright 2021-2024 PIConGPU contributors


PrometheusPi · 2025-05-26T08:30:57Z

@mafshari64 There seems to be conflicts with the current PR. See info.

chillenzer

Offline discussion: We need further testing before approval.

chillenzer · 2025-05-20T08:59:27Z

lib/python/picongpu/picmi/diagnostics/__init__.py

-"""
-This file is part of PIConGPU.
-Copyright 2024 PIConGPU contributors
-Authors: Julian Lenz, Masoud Afshari
-License: GPLv3+
-"""
-


Why's this removed?

chillenzer · 2025-05-20T09:00:47Z

lib/python/picongpu/picmi/diagnostics/__init__.py

 from .timestepspec import TimeStepSpec
 from .checkpoint import Checkpoint
+from .openpmd import OpenPMD
+from .openpmd_sources.source_base import SourceBase


I don't think that this should be here. We don't want to expose this to the user.

so delete it?

chillenzer · 2025-05-20T09:04:30Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

+    @param range contiguous range of cells to dump the base- and derived field for
+        specification as comma-separated "begin:end" range for each dimension, i.e. "begin:end,begin:end,begin:end"
+        Notes: Values will be clipped to the simulation box. Begin and/or end may be omitted to indicate the limit of the simulation box in the dimension. The Default value ":,::,:" indicates that fields should be dumped for all cells.


This should become a class similar to TimeStepSpec. But be aware that there are subtle differences in semantics that would need to be represented in that new class.

chillenzer · 2025-05-20T10:39:53Z

lib/python/picongpu/picmi/diagnostics/openpmd.py

+        if self.period is None:
+            raise ValueError("period is mandatory")


This can't happen.

Do you mean should I delete it?

I retained for defensive programming probably in future other external users might subclass or modify behavior in unexpected ways. but if you want I can remove it.

mafshari64 · 2025-08-04T12:09:27Z

Hi Julian,

@chillenzer, I have finally solved the issues and finalized this branch. In a separate PR I will add all CI tests for picim and pypicongpu. agree?

PrometheusPi · 2025-08-15T10:58:30Z

@mafshari64 Please rebase your PR to the current dev. The branch has conflicts.

PrometheusPi · 2025-08-20T12:23:18Z

@mafshari64 I do not see a fix of this PR yet, do you have problems resolving the conflict? This feature would be great to have in dev.

chillenzer · 2025-09-02T11:51:52Z

Superseded by #5461 which includes these commits.

BrianMarre suggested changes Apr 28, 2025

View reviewed changes

chillenzer added CI:no-compile CI is skipping compile/runtime tests but runs PICMI tests PICMI pypicongpu and picmi related labels Apr 28, 2025

chillenzer added this to the 0.9.0 / next stable milestone Apr 28, 2025

chillenzer self-requested a review April 28, 2025 14:41

BrianMarre suggested changes Apr 29, 2025

View reviewed changes

BrianMarre suggested changes Apr 30, 2025

View reviewed changes

chillenzer mentioned this pull request May 5, 2025

Add openPMD to PICMI #5294

Closed

3 tasks

PrometheusPi requested changes May 6, 2025

View reviewed changes

chillenzer changed the title ~~openpmd.py pluging for PICMI~~ openpmd.py plugin for PICMI May 20, 2025

chillenzer requested changes May 26, 2025

View reviewed changes

mafshari64 added 15 commits August 4, 2025 15:15

adding openpmd.py and charge_density.py

e4273b1

common interface for all the current sources of the openPMD plugin

79478af

adding species to openpmd sources

62d4c43

adding other openpmd picongpu sources

abcc8ac

providing openpmd sources for pypicongpu

2517139

adding schema for openpmd plugin

584e3a1

updating openpmd files

4e5b3f3

finalizing openpmd plugin

0cba2e5

adding rangespec class to openpmd

4f7a275

editing pypicongpu/output/openpmd.py

b44bf96

removing Default simulation_box to fix CI LWFA

f2da5e4

add simulation_box to s.plugin in picmi.simulation.py

e914825

Fix TypeError in PhaseSpace and Auto by adding simulation_box parameter

9f6e5a1

Fix TypeError for openpmd plugin

8f94e58

Fix simulation_box usage in simulation.py to resolve ruff F841 error

b7c3c53

mafshari64 force-pushed the topic-openpmd branch from b753779 to b7c3c53 Compare August 4, 2025 13:42

chillenzer closed this Sep 2, 2025

		"""
		Specifies the parameters for the openPMD plugin in PIConGPU simulations.

	Copyright 2021-2024 PIConGPU contributors
	Copyright 2021-2025 PIConGPU contributors

		if self.period is None:
		raise ValueError("period is mandatory")

openpmd.py plugin for PICMI #5353

openpmd.py plugin for PICMI #5353

Uh oh!

Conversation

mafshari64 commented Apr 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BrianMarre commented Apr 28, 2025

Uh oh!

BrianMarre left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BrianMarre left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PrometheusPi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PrometheusPi commented May 26, 2025

Uh oh!

chillenzer left a comment