WarpX icon indicating copy to clipboard operation
WarpX copied to clipboard

RZ PSATD: set `update_with_rho=0` as default

Open NeilZaim opened this issue 1 year ago • 8 comments

We do this because simulations with high-density plasmas and update_with_rho=true get very noisy.

NeilZaim avatar Apr 26 '23 09:04 NeilZaim

As expected, this breaks some automated tests. From more worrying to less worrying, we have:

  • The pml_psatd_rz test, for which the maximum electric field at the end of the simulation is more than 2 orders of magnitude higher than the set tolerance. Is it possible that using update_with_rho=0 breaks the PML?
  • The galilean_rz_psatd_current_correction test, for which the error on charge conservation is twice higher than the set tolerance.
  • The galilean_rz_psatd test has benchmark changes on the 1-10% level for most field quantities.
  • Two other tests fail with very small benchmark changes.

For people who know these tests, what do you think we should do? cc. @EZoni @RemiLehe

NeilZaim avatar Apr 26 '23 12:04 NeilZaim

Is update_with_rho needed under the same conditions as Cartesian, i.e. true with Galilean? Would that be affecting the two CI tests?

dpgrote avatar Apr 28 '23 20:04 dpgrote

@dpgrote @NeilZaim For the Galilean tests, I think Dave's point is a good one. In Cartesian geometry, we set update_with_rho to true by default for Galilean PSATD because we had observed that it was giving "more correct" results in the test cases we tried, even though the reason is quite unclear at this point.

I think I would be in favor of setting update_with_rho=1 in the runtime_params of those tests so that we don't have to change the benchmarks and we also implicitely flag that it's a good idea to have it ON with Galilean PSATD.

Alternatively we could discuss setting it to true by default for Galilean only, but I think our sample of test cases to choose one or the other is not too large.

For the PML test, the situation is less clear and we would probably need to investigate it more.

EZoni avatar Apr 28 '23 21:04 EZoni

Regarding the PML test, I wonder how much the absolute tolerance tolerance_abs = 2., which is set empirically in the analysis, is expected to change, itself, due to the different algorithm chosen by setting update_with_rho=0.

There is no charge density in the PML equations, so I'm not sure what could be causing an issue otherwise.

EZoni avatar Apr 28 '23 22:04 EZoni

Could the test for Galilean for Cartesian also be used here? I.e. remove the # ifndef WARPX_DIM_RZ so the check is done for all cases.

dpgrote avatar Apr 28 '23 23:04 dpgrote

@NeilZaim I think I agree with Dave here. Since the issue you observed is related to standard PSATD only, let's fix it step by step. We can try to keep psatd.update_with_rho = true by default for the Galilean case in RZ geometry too, as it is in Cartesian geometry. What do you think? The issue with the PML test is separate and it would still need to be understood.

EZoni avatar May 05 '23 20:05 EZoni

Sorry for the slow reaction. I've done as you suggested and now only the pml_psatd_rz fails significantly.

NeilZaim avatar Jun 09 '23 08:06 NeilZaim

I pushed a few commits to update this old branch and see if we can still get these changes to work.

EZoni avatar Sep 16 '24 22:09 EZoni