Throwing a warning if particle_shape>1 with EB #2592

lgiacome · 2021-11-22T18:18:30Z

Following the discussion on slack, we disable the possibility to use EB with particle_shape != 1 because it would require special care for the field gather.

RemiLehe

Would you be okay with raising a warning instead of an abort here?

The reason is that, although the gather/deposition close to the EB is indeed not 100% correct with particle_shape > 1, my impression is that many simulated scenarios will not be affected. (e.g. scenarios where particles do not touch the boundaries, such as well-focused beams in an accelerating cavity).

On the other hand, using particle_shape = 1 is problematic in many cases, especially when using the default energy-conserving scheme, which in this case uses nearest-grid-point interpolation in some directions to gather the fields.

lgiacome · 2021-11-22T20:12:36Z

@RemiLehe your point makes total sense to me so I switched to a warning. Thanks a lot for the suggestion.

lgiacome · 2021-11-22T20:21:39Z

@RemiLehe sorry I somehow missed this part of your comment

On the other hand, using particle_shape = 1 is problematic in many cases, especially when using the default energy-conserving scheme, which in this case uses nearest-grid-point interpolation in some directions to gather the fields.

Very fair point, but, as far as I understand, this is general and not really related to EB, am I right?

RemiLehe · 2021-11-22T20:56:35Z

Yes, that's correct, the issues with particle_shape = 1 are general and not specific to EB.
I guess the point is that, because of these issues, we don't want to impose particle_shape = 1 even for specific options (like EB).

lgiacome · 2021-11-22T21:13:50Z

Sure, I understand and fully agree.

ax3l · 2021-11-22T23:13:47Z

Source/WarpX.cpp

@@ -959,6 +959,14 @@ WarpX::ReadParameters ()
                    noy = particle_shape;
                    noz = particle_shape;
                }
+#ifdef AMREX_USE_EB
+                if (particle_shape > 1)


Please add here that embedded boundaries are also enabled at runtime, before raising this.

Does this affect EBs for ECT and ES solver? Otherwise, we should also check for the exact field solver allow-list/warn-list here.

@ax3l
I added the runtime check, thanks a lot for the suggestion. I had to move the warning because where it used to be the EB data are not initialized yet.

Does this affect EBs for ECT and ES solver? Otherwise, we should also check for the exact field solver allow-list/warn-list here.

This affects EB with every possible solver (YEE, CKC, ECT, ES..) so I don't think we need any check for the solver here.

Source/Initialization/WarpXInitData.cpp

ax3l

LGTM, thank you! 🎉

…ning

Addressed :)

* Fix Init of Vector Members (ECP-WarpX#2595) Fix default init of `Vector` member variables. The old construct is not valid C++. https://stackoverflow.com/a/11491003/2719194 * C++17: Work-Around NVCC gatherParticles (ECP-WarpX#2596) The `noexcept` lambda does not compile in C++17 mode due to an NVCC compiler bug, at least in NVCC 11.3.109. Compiles in C++14 mode with the same compiler. * requirements.txt - PICMI development version (ECP-WarpX#2588) Document in `requirements.txt` on how to install a pre-release version of PICMI. * CONTRIBUTING: Update/Modernize (ECP-WarpX#2600) * CONTRIBUTING: Update/Modernize - Add GitHub account setup - Add local git setup - Modernize for CMake workflows * Apply suggestions by Edoardo Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com> * Replaced duplicated current deposition documentation (ECP-WarpX#2604) * Throwing a warning if particle_shape>1 with EB (ECP-WarpX#2592) * Aborting if particle_shape!=1 with EB * Throw warning instead of aborting * Checking at runtime if EB is initialized * Added missing preprocessor directive * Ignoring an unused variable * Fix typo * Improve style Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja> * Fix segfault when importing _libwarpx without initializing WarpX (ECP-WarpX#2580) * Added check for if warpx was initialized when calling finalize * Renamed to be warpx_initialized * Fixed reference to global variable Co-authored-by: Peter Scherpelz <31747262+peterscherpelz@users.noreply.github.com> * Changed global variable to member of libwarpx * Fixed syntax errors * Remove custom arg from argv to avoid parmparse error Co-authored-by: Peter Scherpelz <31747262+peterscherpelz@users.noreply.github.com> * Added parallel pragma to ApplyBoundaryConditions (ECP-WarpX#2612) * Note that CCache 4.2 introduced large CUDA improvements (ECP-WarpX#2606) * Dimensionality Docs: Default (ECP-WarpX#2609) Just adds the note that 3D is the default geometry. * AMReX: Weekly Update (ECP-WarpX#2613) * MergeBuffersForPlotfile: Barrier (ECP-WarpX#2608) Make sure that all MPI ranks are in sync, i.e., have closed the files that they wrote, before trying to merge them. * Fix installation location for libraries (ECP-WarpX#2583) During configuration the installation location for libraries is given by dumping the cmake variable `CMAKE_INSTALL_LIBDIR`. This commit adjusts the installation of WarpX libraries (WarpX_LIB=ON) to respect this setting. Co-authored-by: Rolf Pfeffertal <tropf@users.noreply.github.com> * Release 21.12 (ECP-WarpX#2614) * AMReX: 21.12 * PICSAR: 21.12 * WarpX: 21.12 * div(E,B) Cleaning Options for PSATD (ECP-WarpX#2403) * Implement div(E)/div(B) Cleaning with Standard PSATD * Cleaning * Update Benchmark * Add Nodal Synchronization of F,G * OneStep_multiJ: Nodal Syncs, Damp PML * OneStep_multiJ: Push PSATD Fields in PML * div Cleaning Defaults (Domain v. PML) * Include Fix of ECP-WarpX#2429 until Merged * Reset Benchmark of Langmuir_multi_psatd_div_cleaning * Multi-J: Remove PML Support * Include Fix of ECP-WarpX#2474 Until Merged * Exchange All Guard Cells for F,G * Fix Defaults * Update Test, Reset Benchmark * Fix Defaults * Cleaning * Default update_with_rho=1 if do_dive_cleaning=1 * Update CI Test pml_psatd_dive_divb_cleaning * Replace Warning with Abort * Add 2D Langmuir Test w/ MR & PSATD (ECP-WarpX#2605) * Add 2D Langmuir Test w/ MR & PSATD * Add Missing Compile String * Fix out-of-bound in Inverse FFT of F,G (ECP-WarpX#2619) * Mention that the potentail should be constant inside EB (ECP-WarpX#2618) * Mention that the potentail should be constant inside EB * Update text * Replace AMREX_SPACEDIM: Boundary & Parallelization (ECP-WarpX#2620) * AMREX_SPACEDIM : Boundary Conditions * AMREX_SPACEDIM : Parallelization * Fix compilation * Update Source/Parallelization/WarpXComm_K.H * Fix out-of-bound in the initialization of EB (ECP-WarpX#2607) * Call FillBoundary when initializing EB * Avoid out-of-bound * Bug fix * Apply suggestions from code review * update version number Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja> Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com> Co-authored-by: Remi Lehe <remi.lehe@normalesup.org> Co-authored-by: Lorenzo Giacomel <47607756+lgiacome@users.noreply.github.com> Co-authored-by: Kevin Z. Zhu <86268612+KZhu-ME@users.noreply.github.com> Co-authored-by: Peter Scherpelz <31747262+peterscherpelz@users.noreply.github.com> Co-authored-by: David Grote <grote1@llnl.gov> Co-authored-by: Phil Miller <phil@intensecomputing.com> Co-authored-by: s9105947 <80697868+s9105947@users.noreply.github.com> Co-authored-by: Rolf Pfeffertal <tropf@users.noreply.github.com> Co-authored-by: Prabhat Kumar <89051199+prkkumar@users.noreply.github.com>

Aborting if particle_shape!=1 with EB

2200d9e

RemiLehe previously requested changes Nov 22, 2021

View reviewed changes

Throw warning instead of aborting

3b867cc

lgiacome changed the title ~~[mini-PR] Aborting if particle_shape!=1 with EB~~ [mini-PR] Throwing a warning if particle_shape>1 with EB Nov 22, 2021

lgiacome added warning A compiler or tool warning and removed warning A compiler or tool warning labels Nov 22, 2021

ax3l added changes input scripts / defaults Changes the syntax or meaning of input scripts and/or defaults component: boundary PML, embedded boundaries, et al. labels Nov 22, 2021

ax3l reviewed Nov 22, 2021

View reviewed changes

ax3l self-assigned this Nov 23, 2021

lgiacome added 4 commits November 23, 2021 10:04

Checking at runtime if EB is initialized

8faaab7

Added missing preprocessor directive

865e9f8

Ignoring an unused variable

6fd84d8

Fix typo

50e2ec3

ax3l requested a review from RemiLehe November 24, 2021 16:29

ax3l reviewed Nov 24, 2021

View reviewed changes

Source/Initialization/WarpXInitData.cpp Outdated Show resolved Hide resolved

ax3l reviewed Nov 24, 2021

View reviewed changes

Source/Initialization/WarpXInitData.cpp Outdated Show resolved Hide resolved

ax3l reviewed Nov 24, 2021

View reviewed changes

Source/Initialization/WarpXInitData.cpp Outdated Show resolved Hide resolved

Improve style

0dfa7aa

ax3l approved these changes Nov 24, 2021

View reviewed changes

ax3l changed the title ~~[mini-PR] Throwing a warning if particle_shape>1 with EB~~ Throwing a warning if particle_shape>1 with EB Nov 24, 2021

Merge remote-tracking branch 'upstream/development' into EB_shape_war…

60072fc

…ning

ax3l merged commit 4273717 into ECP-WarpX:development Nov 30, 2021

lgiacome deleted the EB_shape_warning branch December 14, 2021 08:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Throwing a warning if particle_shape>1 with EB #2592

Throwing a warning if particle_shape>1 with EB #2592

lgiacome commented Nov 22, 2021

RemiLehe left a comment

lgiacome commented Nov 22, 2021

lgiacome commented Nov 22, 2021

RemiLehe commented Nov 22, 2021 •

edited

Loading

lgiacome commented Nov 22, 2021 •

edited

Loading

ax3l Nov 22, 2021 •

edited

Loading

lgiacome Nov 23, 2021

ax3l left a comment

Throwing a warning if particle_shape>1 with EB #2592

Throwing a warning if particle_shape>1 with EB #2592

Conversation

lgiacome commented Nov 22, 2021

RemiLehe left a comment

Choose a reason for hiding this comment

lgiacome commented Nov 22, 2021

lgiacome commented Nov 22, 2021

RemiLehe commented Nov 22, 2021 • edited Loading

lgiacome commented Nov 22, 2021 • edited Loading

ax3l Nov 22, 2021 • edited Loading

Choose a reason for hiding this comment

lgiacome Nov 23, 2021

Choose a reason for hiding this comment

ax3l left a comment

Choose a reason for hiding this comment

RemiLehe commented Nov 22, 2021 •

edited

Loading

lgiacome commented Nov 22, 2021 •

edited

Loading

ax3l Nov 22, 2021 •

edited

Loading