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.

Sign up for GitHub

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

Jump to bottom

RZ + openPMD : don't transpose data, save data with shape (Nm, Nz, Nr) #3030

Merged
merged 8 commits into from
Apr 12, 2022
Merged

RZ + openPMD : don't transpose data, save data with shape (Nm, Nz, Nr) #3030

merged 8 commits into from
Apr 12, 2022

Conversation

RTSandberg
Copy link
Member

@RTSandberg RTSandberg commented Apr 7, 2022 •
edited
Loading

Fixing PR #2878

Code transposition is turned off
Data is stored in the shape (Nm, Nz, Nr) rather than (Nm, Nr, Nz).

In PR #2878, a choice was made to transpose data on CPU to adhere to the openPMD standard requiring data in the shape (Nm, Nr, Nz). However, the data shape was incorrect for the GPU untransposed case. Discussing with the OpenPMD developers, the OpenPMD standard will be relaxed to allow data of the shape (Nm, Nz, Nr); see openPMD/openPMD-viewer#337 This means no transposing is necessary

@RTSandberg RTSandberg requested review from ax3l, guj and RemiLehe April 7, 2022 21:42
@ax3l ax3l self-assigned this Apr 8, 2022
@ax3l ax3l requested a review from camille12225 April 8, 2022 00:55
@ax3l ax3l added Performance optimization component: openPMD openPMD I/O geometry: RZ axisymmetric 2D and quasi-3D labels Apr 8, 2022
@ax3l
Copy link
Member

ax3l commented Apr 8, 2022 •
edited
Loading

Hi @RTSandberg, thanks a lot.

In order to finalize this, can you please link the PR to openPMD-viewer that handles the data reconstruction depending on the value in axisLabels for RZ accordingly? (time sensitive, since @camille12225 depends on this)

We also need to update the openPMD standard together to document this relaxation :)
(less timely, e.g., next week also ok)

@RemiLehe
Copy link
Member

RemiLehe commented Apr 8, 2022

Thanks for this PR @RTSandberg !
Since we are not doing the transposition anymore, would it make sense to clean up the variable do_transpose, and remove the transposition functions (transposeChunk) from the code altogether?

@RTSandberg
Copy link
Member Author

@RemiLehe Yes, I can remove do_transpose and refactor. I wanted to make sure everyone is ok with this decision before removing the transposing code

RemiLehe
RemiLehe approved these changes Apr 8, 2022
View reviewed changes
Source/Diagnostics/WarpXOpenPMD.cpp Outdated Show resolved Hide resolved
@ax3l ax3l mentioned this pull request Apr 11, 2022
Open
@ax3l ax3l force-pushed the topic-RZopenPMDFlexTranspose branch 3 times, most recently from 7208020 to b855942 Compare April 12, 2022 01:19
@ax3l ax3l force-pushed the topic-RZopenPMDFlexTranspose branch from b855942 to c76bd44 Compare April 12, 2022 01:21
ax3l
ax3l reviewed Apr 12, 2022
View reviewed changes
Source/Diagnostics/WarpXOpenPMD.cpp
@@ -1459,20 +1414,8 @@ WarpXOpenPMDPlot::WriteOpenPMDFieldsAll ( //const std::string& filename,
#endif
{
amrex::Real const *local_data = fab.dataPtr(icomp);
#ifdef WARPX_DIM_RZ
Copy link
Member

@ax3l ax3l Apr 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Note: this block was only taking the CPU path, and we forgot to transpose on GPU. With this PR (+ openPMD/openPMD-viewer#337), we don't need to anymore anyway :)

ax3l
ax3l approved these changes Apr 12, 2022
View reviewed changes
Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

LGTM, thank you! ✨
I added a little clean-up.

@ax3l ax3l enabled auto-merge (squash) April 12, 2022 01:28
@ax3l ax3l merged commit c7e4c9d into ECP-WarpX:development Apr 12, 2022
roelof-groenewald added a commit to ModernElectron/WarpX that referenced this pull request Apr 20, 2022
@roelof-groenewald @ax3l
@EZoni @pre-commit-ci @RTSandberg @RemiLehe @WeiqunZhang @Yin-YinjianZhao @KZhu-ME
Update memaster to upstream e538399 (#174)
7c24547 
* Signal: Cleanup (Simplify) (ECP-WarpX#3031)

Simplify the logic by fusing a loop.

* Fix Bug in `WarpX::MoveWindow` (ECP-WarpX#3034)

* [pre-commit.ci] pre-commit autoupdate (ECP-WarpX#3039)

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.1.0 → v4.2.0](pre-commit/pre-commit-hooks@v4.1.0...v4.2.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* AMReX: Update to latest commit (ECP-WarpX#3037)

* RZ + openPMD : don't transpose data, save data with shape (Nm, Nz, Nr) (ECP-WarpX#3030)

* part_per_grid/cell : cartesian diag in RZ/openPMD

* RZ+openPMD : flexible ZR or RZ order

* fix logic if not in RZ

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

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

* fix regression test, remove transpose and refactor

* Update Source/Diagnostics/WarpXOpenPMD.cpp

* Fix: Doxygen, Consistent Naming, Comments, Style

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Remi Lehe <remi.lehe@normalesup.org>
Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

* Remove Nodal Synchronization of J and Rho (ECP-WarpX#3038)

* Remove Nodal Sync of J and Rho

* Reset Benchmark of reduced_diags_single_precision

* Avoid unnecessary construction of CopyParticleAttribs (ECP-WarpX#3042)

If it's not used there is no reason to construct a CopyParticleAttribs
object.  In fact, it could result in a runtime `std::out_of_range` error in
`std::map::at()` because `tmp_particle_data` is used in the
CopyParticleAttribs constructor, but in `PhysicalParticleContainer::Evolve`,
`tmp_particle_data` is properly prepared only when doing back transformed
diagnostics.

* AMReX/PICSAR: Weekly Update (ECP-WarpX#3046)

* [pre-commit.ci] pre-commit autoupdate (ECP-WarpX#3049)

updates:
- [github.com/hadialqattan/pycln: v1.2.5 → v1.3.1](hadialqattan/pycln@v1.2.5...v1.3.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Summit: Prepend Hostname & Mapping (ECP-WarpX#3040)

Add an additional `jsrun` command on Summit so that one can easier map the run output to a hostname. Write a `task_host_mapping.txt` file for each run.

* Fix a bug about particle weight. (ECP-WarpX#3052)

Co-authored-by: Yinjian Zhao <yin@Yinjians-MacBook-Air.local>

* add after diagnostic python callback (ECP-WarpX#3043)

* Fix total step bug (#173)

* don't decrement total_steps

* Update version number and changelog

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

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

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* After diagnostic callback (#172)

* add after diagnostic python callback

* use afterdiagnostics callback to save flux diag checkpoint files

* fix failing unit test

* changes requested during PR review

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Ryan Sandberg <RSandberg@lbl.gov>
Co-authored-by: Remi Lehe <remi.lehe@normalesup.org>
Co-authored-by: Weiqun Zhang <WeiqunZhang@lbl.gov>
Co-authored-by: Yinjian Zhao <yinjianzhao@lbl.gov>
Co-authored-by: Yinjian Zhao <yin@Yinjians-MacBook-Air.local>
Co-authored-by: Kevin Z. Zhu <86268612+KZhu-ME@users.noreply.github.com>
RevathiJambunathan pushed a commit to RevathiJambunathan/WarpX that referenced this pull request May 3, 2022
@hklion @EZoni
@ax3l @PhilMiller @roelof-groenewald @ElisaRheaume @pre-commit-ci @WeiqunZhang @dpgrote @lgiacome @RTSandberg @RemiLehe @Yin-YinjianZhao
HarrisSheetinX-development merge 22-04-29 (#90)
c562208 
* CI: Fix Small Issue w/ Langmuir Plots (ECP-WarpX#3010)

* Release 22.04 (ECP-WarpX#3018)

* AMReX: 22.04

* PICSAR: 22.04

* WarpX: 22.04

* Switch test to use common build with openPMD enabled (ECP-WarpX#2988)

* Switch test to use common build with OpenPMD enabled

* Match GNU make to CMake change

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

* Allow process signals to trigger checkpoint or break behavior (ECP-WarpX#2896)

* Beginnings of signal handling machinery

* Add tentative logic to make checkpoint call

* Adapt formatting slightly

* Add calls to read signals and set up signal handlers

* Initialize signal flag array

* Add parsing of signal names, and fix some whitespace issues

* Skip signal setup on Windows

* added checkpoint and break signal inputs to picmi.py

* Address initial review requests

* Correct comment to match changed code

* Convert maximum signal number to a symbolic name

* Always parse signal input, and error out on Windows or wherever it may be unsupported

* Typo fix

* Add missing reset of checkpoint signal flag

* Add reset of break signal, in support of Python or library usage

* Test for a configured checkpoint diag when asked to checkpoint on a signal

* Fix typo in Linux code path

* Clean up MPI support

* Use symbolic name for maximum signal number

* Fix unused variable in the no-MPI case

* Add missing header inclusions

* Switch signal parsing to an enumerated table

* Test signal handling for Linux, not GNU C library

* Avoid another magic number

* Update MPI_Ibcast call to match symbolic array length

* Update loop over signal flags to use symbolic limit

* Match #includes to usage

* Add omitted C++ std <atomic> header include

* Guard entire set of signal definitions as *nix-only, not for Windows

* Broaden Windows exclusion to avoid zero-length array that displeases MSVC++

* Check return value from sigaction()

* Convert conditional calls to Abort() to assertions

* Move check for platform support to input parsing

* Shift signal handling code over toward ABLASTR to share with ImpactX and Hipace++

* Minor cleanup

* A bit more cleanup

* Fix formatting nits

* Add AMReX error handling on MPI calls

* Add ABLASTR signal handling code to GNU makefile too

* Document new input parameters

* Use ABLASTR assertion macros in ABLASTR code

* Convert requests limit value to a requests array size

* Generalize signal handling to an arbitrary set of potential actions

* Rename class to match usage and file name

* Stick stuff in ABLASTR namespace

* Indent conditional includes as requested

Co-authored-by: Roelof <roelof.groenewald@modernelectron.com>

* Moving Frame Field Probe Functionality (ECP-WarpX#2996)

* DRAFT for Moving Frame

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

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

* Adds m_last_compute_step

* Generalized SetParticlePosition, semicolons and formatting

* Update Source/Diagnostics/ReducedDiags/FieldProbe.H

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

* Moved particle push before calculations. Condensed stuff.

* Fixed Velocity, added start and stop time functionality

* Documentation

* Apply suggestions from code review

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

* Empty-Commit

* Fixed ParallelFor from review

* FP moving window in laser_acceleration 1d, 2d, 3d test

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

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

* Line detector instead of point

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

* Refactoring: More General Interfaces (ECP-WarpX#2991)

* Refactoring: More General Interfaces (J, rho)

* Refactoring: More General Interfaces (E, B)

* Refactoring: More General Interfaces (rho)

* Fix const Correctness

* Fix const Correctness (continued)

* Fix Bugs

* Refactoring: More General Interfaces (averaged E, averaged B)

* LoadBalanceCosts: num_cells & num_macro_particles (ECP-WarpX#3019)

* Examples: LBC in Laser-Ion Example

* LoadBalanceCosts: num_cells & num_macro_particles

* Fix Script: Wrong Header Counting

Spliced out the first to columns twice.

* Fix Script: Blocking Factor

The script did not work if a direction only had one block

* Fix Script: 2D/3D

There is no generic way from the data we write to find this out,
without breaking corner cases such as one-block in one direction.

* LoadBalanceCosts: Fix Unused Vars in Script

Leftover to ECP-WarpX#3019

* Semicoarsening in electrostatic solvers (ECP-WarpX#3024)

* Semicoarsening in electrostatic solvers

Enable semicoarsening in electrostatic solvers if the problem is anisotropic
due to relativistic beam and/or anisotropic cell spacing.

* Update relativistic_space_charge_initialization benchmark

* AMReX: Update to latest commit (ECP-WarpX#3021)

* Added PlasmaLens class to PICMI (ECP-WarpX#3025)

* Added PlasmaLens class to PICMI

* Added CI test

* Added PICMI input file

* Fixed the output dir for the CI test

* Add `_plt` to Output File Prefix

Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com>

* Minor fixes removing warnings from MSVC C++ (ECP-WarpX#3022)

* Fix MPI Signal Handling on Perlmutter (ECP-WarpX#3029)

* Fix: `MPI_CXX_BOOL` -> `MPI_BYTE`

C99 types were aded in MPI-2.2, while Cray's MPICH fork in version
8.1.13 defines `MPI_CXX_BOOL` to `MPI_DATATYPE_NULL` on Perlmutter.

We could use `MPI_C_BOOL`, which is technically a `_Bool` from
[<cstdbool>](https://en.cppreference.com/w/cpp/header/cstdbool)
(deprecated: C++17; removed: C++20) - or we simply do a static
assert on `sizeof(bool)` and communicate as a `MPI_BYTE` or
`MPI_CHAR`.

* Signals: Do no MPI Comms if none is configured

* Docs: Link and Explain Allowed Signal Values

* Review Comments & Style

* Extend `do_pml_Lo/Hi` to MR Levels (ECP-WarpX#2890)

* Vay Deposition: Fix SyncCurrent, FFTs (ECP-WarpX#3012)

* Implemented the BCK fallback for the ECT solver (ECP-WarpX#3016)

* Implemented the BCK fallback for the ECT solver

* Ignored an unused variable

* Bug fix

* Fix issue with preprocessor directive

* Added comments

* Implemented suggestions from code review

* Fixed tilebox

* Improved warning

* Using WarpX::RecordWarning instead of amrex::Print()

* Ignoring RZ

* Improving comment

* Bug Fix

* Lowered warning priority

* Ignoring low priority warnings in ECT tests

Co-authored-by: lgiacome <lorenzo.giacome@cern.ch>

* Signal: Cleanup (Simplify) (ECP-WarpX#3031)

Simplify the logic by fusing a loop.

* Fix Bug in `WarpX::MoveWindow` (ECP-WarpX#3034)

* [pre-commit.ci] pre-commit autoupdate (ECP-WarpX#3039)

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.1.0 → v4.2.0](pre-commit/pre-commit-hooks@v4.1.0...v4.2.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* AMReX: Update to latest commit (ECP-WarpX#3037)

* RZ + openPMD : don't transpose data, save data with shape (Nm, Nz, Nr) (ECP-WarpX#3030)

* part_per_grid/cell : cartesian diag in RZ/openPMD

* RZ+openPMD : flexible ZR or RZ order

* fix logic if not in RZ

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

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

* fix regression test, remove transpose and refactor

* Update Source/Diagnostics/WarpXOpenPMD.cpp

* Fix: Doxygen, Consistent Naming, Comments, Style

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Remi Lehe <remi.lehe@normalesup.org>
Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

* Remove Nodal Synchronization of J and Rho (ECP-WarpX#3038)

* Remove Nodal Sync of J and Rho

* Reset Benchmark of reduced_diags_single_precision

* Avoid unnecessary construction of CopyParticleAttribs (ECP-WarpX#3042)

If it's not used there is no reason to construct a CopyParticleAttribs
object.  In fact, it could result in a runtime `std::out_of_range` error in
`std::map::at()` because `tmp_particle_data` is used in the
CopyParticleAttribs constructor, but in `PhysicalParticleContainer::Evolve`,
`tmp_particle_data` is properly prepared only when doing back transformed
diagnostics.

* AMReX/PICSAR: Weekly Update (ECP-WarpX#3046)

* [pre-commit.ci] pre-commit autoupdate (ECP-WarpX#3049)

updates:
- [github.com/hadialqattan/pycln: v1.2.5 → v1.3.1](hadialqattan/pycln@v1.2.5...v1.3.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Summit: Prepend Hostname & Mapping (ECP-WarpX#3040)

Add an additional `jsrun` command on Summit so that one can easier map the run output to a hostname. Write a `task_host_mapping.txt` file for each run.

* Fix a bug about particle weight. (ECP-WarpX#3052)

Co-authored-by: Yinjian Zhao <yin@Yinjians-MacBook-Air.local>

* add after diagnostic python callback (ECP-WarpX#3043)

* Current Deposition Interfaces: Always Pass `dt` & `relative_time` (ECP-WarpX#2937)

* Current Deposition Interfaces: Always Pass `dt` & `relative_time`

* Fix CI Failures

* Fix 1D/RZ Warnings

* Fix RZ Bug

* Fix Bugs

* Minimize Changes, Fix Roundoff Errors

* Cleaning

* Remove dt From doDepositionShapeN

* Cleaning Esirkepov Deposition

* Cleaning Vay Deposition

* Fix Roundoff Errors

* Fix Roundoff Errors (continued)

* Fix Bug (RZ)

* `Stencil.py`: Follow PEP 8 Coding Style (ECP-WarpX#3055)

* `Stencil.py`: Follow PEP 8 Coding Style

* Commit Suggestion by @dpgrote

* Perlmutter: Boost Software Module (QED) (ECP-WarpX#3053)

* Perlmutter: Boost

Document a compatible Boost module on Perlmutter.
This is needed for detailed table generation for the QED module
(`WarpX_QED_TABLE_GEN=ON`).

* CMake: Update PICSAR

Include a fix for a compile error on CPU for QED table gen.

* Docs: Ascent 0.8.0 on Summit (ECP-WarpX#3057)

Update the Ascent installation location to use Cyrus' new install
for us.

* AMReX: Weekly Update (ECP-WarpX#3059)

* Doc: PICMI HPC System Update Requirements (ECP-WarpX#3060)

Since these requirements (i.e., PICMI) change quite rapidly, document
the line close to the `pip` install line.

* Add profiling to ParticleBoundaryBuffer::gatherParticles (ECP-WarpX#3061)

This turned out to be a major portion of a particle-heavy simulation,
and was being entirely attributed to WarpX::Evolve().

* Update CUDA repo key (ECP-WarpX#3069)

Nvidia has made changes in the signing keys.

https://forums.developer.nvidia.com/t/notice-cuda-linux-repository-key-rotation/212771

Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com>
Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
Co-authored-by: Phil Miller <unmobile+gh@gmail.com>
Co-authored-by: Roelof <roelof.groenewald@modernelectron.com>
Co-authored-by: Tiberius Rheaume <35204125+TiberiusRheaume@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Weiqun Zhang <WeiqunZhang@lbl.gov>
Co-authored-by: David Grote <grote1@llnl.gov>
Co-authored-by: Lorenzo Giacomel <47607756+lgiacome@users.noreply.github.com>
Co-authored-by: lgiacome <lorenzo.giacome@cern.ch>
Co-authored-by: Ryan Sandberg <RSandberg@lbl.gov>
Co-authored-by: Remi Lehe <remi.lehe@normalesup.org>
Co-authored-by: Yinjian Zhao <yinjianzhao@lbl.gov>
Co-authored-by: Yinjian Zhao <yin@Yinjians-MacBook-Air.local>
Co-authored-by: Roelof Groenewald <40245517+roelof-groenewald@users.noreply.github.com>
Co-authored-by: Phil Miller <phil@intensecomputing.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RemiLehe RemiLehe RemiLehe approved these changes

@ax3l ax3l ax3l approved these changes

@guj guj Awaiting requested review from guj

@camille12225 camille12225 Awaiting requested review from camille12225

Assignees

@ax3l ax3l

Labels
component: openPMD openPMD I/O geometry: RZ axisymmetric 2D and quasi-3D Performance optimization
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RTSandberg @ax3l @RemiLehe