Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use serial openPMD #876

Merged
merged 8 commits into from
Mar 30, 2023

Conversation

SeverinDiederichs
Copy link
Member

We compile openPMD with the or without MPI according to whether HiPACE++ itself is compiled with or without MPI. However, IO is always serial, so this is not needed.

Ran into a compilation issue with ADIOS2 with MPI on LUMI, there I noticed this.

  • Small enough (< few 100s of lines), otherwise it should probably be split into smaller PRs
  • Tested (describe the tests in the PR description)
  • Runs on GPU (basic: the code compiles and run well with the new module)
  • Contains an automated test (checksum and/or comparison with theory)
  • Documented: all elements (classes and their members, functions, namespaces, etc.) are documented
  • Constified (All that can be const is const)
  • Code is clean (no unwanted comments, )
  • Style and code conventions are respected at the bottom of https://github.com/Hi-PACE/hipace
  • Proper label and GitHub project, if applicable

@SeverinDiederichs SeverinDiederichs added the build CMake, compilation, etc. label Feb 28, 2023
@SeverinDiederichs SeverinDiederichs changed the title use serial openPMD [WIP] use serial openPMD Mar 1, 2023
@SeverinDiederichs SeverinDiederichs changed the title [WIP] use serial openPMD use serial openPMD Mar 14, 2023
@ax3l ax3l self-requested a review March 14, 2023 16:18
@ax3l ax3l self-assigned this Mar 14, 2023
@@ -11,7 +11,7 @@ function(find_openpmd)
set(CMAKE_POLICY_DEFAULT_CMP0077 NEW)

# see https://openpmd-api.readthedocs.io/en/0.14.2/dev/buildoptions.html
set(openPMD_USE_MPI ${HiPACE_MPI} CACHE INTERNAL "")
set(openPMD_USE_MPI OFF CACHE INTERNAL "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this an option.

The reason for that is: let's assume you want to use HiPACE++ in a software environment with other BLAST codes that use openPMD, too. Some might depend on a parallel HDF5. But HDF5 unfortunately cannot be installed in two binary variants at the same time in environments like conda.

Similarly, an HPC system might only provide us with a parallel HDF5 (or only a serial HDF5) module.

In such situations, we want to control the parallelism of the I/O layer, even if we only use it serially.

Simple solution that I prefer:
we add an option(...).

Fancy solution:
we do a find_package(HDF5 ...) and decide based on this (but this is more involved and does not apply to other backends like ADIOS2)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, let's do the simple solution. Will update the PR when I have time :)
Thanks!

Copy link
Member

@MaxThevenet MaxThevenet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR!

@MaxThevenet MaxThevenet merged commit 8e84fe5 into Hi-PACE:development Mar 30, 2023
@SeverinDiederichs SeverinDiederichs deleted the use_serial_openpmd branch March 31, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build CMake, compilation, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants