Fixing bug with the ES solver and MR: E_aux[lev] = E_fp[lev] in the UpdateAuxilaryData #4922
Conversation
…lgorithms with PML BC
for more information, see https://pre-commit.ci
ax3l
added
bug
…y_update_es_solver
for more information, see https://pre-commit.ci
…y_update_es_solver
…/oshapoval/WarpX into mr_fix_auxilary_update_es_solver
for more information, see https://pre-commit.ci
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…lysis script with smaller MR patch.
…y_update_es_solver
…/oshapoval/WarpX into mr_fix_auxilary_update_es_solver
for more information, see https://pre-commit.ci
…/oshapoval/WarpX into mr_fix_auxilary_update_es_solver
Source/Parallelization/WarpXComm.cpp
Outdated
| if (do_pml && pml[lev]->ok()) | ||
| { | ||
| WARPX_ABORT_WITH_MESSAGE("Averaged Galilean PSATD with PML is not yet implemented"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Just double checking, are this and the following similar changes (I counted four of them) related to this PR? The PR is a bug fix for the electrostatic solver with mesh refinement, while these changes are removing abort statements related to averaged Galilean PSATD with PMLs. Were they maybe left over from a different branch and/or should they be implemented in a dedicated PR?
| amrex::IntVect(0), | ||
| amrex::IntVect(1), |
There was a problem hiding this comment.
I noticed that resetting the first argument (corresponding to src_nghost in the definition of ParallelCopy) to amrex::IntVect(1), as it was before, fixes the "erroneous arithmetic operation" error that was occurring in the tests ElectrostaticSphereLabFrame_MR_emass_10 and ElectrostaticSphereEB_RZ_MR.
What was the reason for deciding to change the code here and pass 0 ghost cells for the source MultiFab, as opposed to 1 ghost cell for the destination MultiFab?
| amrex::IntVect(0), | |
| amrex::IntVect(1), | |
| amrex::IntVect(1), | |
| amrex::IntVect(1), |
|
I'm investigating the failure of the scraping CI tests (which appears as "erroneous arithmetic operation") and what I can report so far is that it probably due to uninitialized values in the MultiFabs It's not clear to me yet why we are leaving some of those values untouched, hence uninitialized, with the code changes here. Regardless of this, we might want to think whether we would like to initialize those MultiFabs to 0.0 as we do for the corresponding MultiFabs on level 0. Merged the code changes in #5049 to verify that they are sufficient to fix the issue. Will rebase once more after #5049 is merged. |
EZoni
force-pushed
the
mr_fix_auxilary_update_es_solver
branch
from
5c7f7dc
to
9407bf9
Compare
|
@oshapoval @RemiLehe |
This PR fixes a bug with the ES solver and MR. This issues was initially observed by @aeriforme. Worked on the issue with @RemiLehe.
Problem: observed doubling of the E field for
lev>0.Solution: when ES solver is used we have
E_cp[levl] =0and we should update the auxiliary data asE_aux[lev] = E_fp[lev](insideWarpX::UpdateAuxilaryDataStagToNodal()andWarpX::UpdateAuxilaryDataSameType()rather thanE_aux[lev] = E_fp[lev] - E_cp[lev] + E_aux[lev-1]in contrast to the EM solver. Otherwise it would result in doubling of E field we observed.To do:
WarpX::UpdateAuxilaryDataStagToNodal ()amr.max_level = 1(reused CI testelectrostatic_sphere_eb_mr)Results after the fix: