Fix charge conservation in charge exchange and add automated test#6796
Conversation
…harge exchange CI test
…hange_2__ci_analysis
…hange_2__ci_analysis
| # verify the total energy conservation | ||
| total_energy_error_norm = np.max(np.abs(total_energy_error)) | ||
| relative_tolerance = 1e-5 if electrostatic else 6e-5 | ||
| relative_tolerance = 1.1e-5 if electrostatic else 6e-5 |
There was a problem hiding this comment.
The new test requires the tolerance to be slightly relaxed.
Co-authored-by: Remi Lehe <remi.lehe@normalesup.org>
for more information, see https://pre-commit.ci
|
|
||
| # compare the final value of the field energy variation to the reference equipartition value | ||
| relative_tolerance = 1e-1 if electrostatic else 5e-1 | ||
| relative_tolerance = 1.1e-1 if electrostatic else 5e-1 |
There was a problem hiding this comment.
For the new test to pass, the tolerance needs to be very slightly increased.
| // Position components occupy indices 0 through PIdx::w-1 in m_rdata. | ||
| if (mask[i] == int(ScatteringProcessType::CHARGE_EXCHANGE)) { | ||
| for (int idim = 0; idim < PIdx::w; ++idim) { | ||
| soa_products_data[2].m_rdata[idim][slot2_idx] = soa_1.m_rdata[idim][src1_idx]; | ||
| soa_products_data[3].m_rdata[idim][slot3_idx] = soa_0.m_rdata[idim][src0_idx]; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
What do you guys think about making this more future proof by using the GetPositionAsStored and SetPositionAsStored structs rather than relying on the position attributes being first in the order?
There was a problem hiding this comment.
In retrospect, since the particle iterators are not themselves easily accessible from this location, I think reverting to this original approach is better (more efficient and more readable). Sorry for the flip-flopping!
There was a problem hiding this comment.
Oh man, @oshapoval, I'm so sorry but I just realized there might be a problem with this approach due to the fact that theta is actually not before w in the attribute list for RZ, RCYLINDER, and RSPHERE - see
I think the balance of things then is to just have some code duplication here that will keep things easy to read. So making this something like:
soa_products_data[2].m_rdata[PIdx::z][slot2_idx] = soa_1.m_rdata[PIdx::z][src1_idx];
soa_products_data[3].m_rdata[PIdx::z][slot3_idx] = soa_0.m_rdata[PIdx::z][src0_idx];
#if defined(WARPX_DIM_RZ)
soa_products_data[2].m_rdata[PIdx::x][slot2_idx] = soa_1.m_rdata[PIdx::x][src1_idx];
soa_products_data[2].m_rdata[PIdx::theta][slot2_idx] = soa_1.m_rdata[PIdx::theta][src1_idx];
soa_products_data[3].m_rdata[PIdx::x][slot3_idx] = soa_0.m_rdata[PIdx::x][src0_idx];
soa_products_data[3].m_rdata[PIdx::theta][slot3_idx] = soa_0.m_rdata[PIdx::theta][src0_idx];
#elif defined(WARPX_DIM_RCYLINDER)
soa_products_data[2].m_rdata[PIdx::theta][slot2_idx] = soa_1.m_rdata[PIdx::theta][src1_idx];
soa_products_data[3].m_rdata[PIdx::theta][slot3_idx] = soa_0.m_rdata[PIdx::theta][src0_idx];
#elif defined(WARPX_DIM_RSPHERE)
soa_products_data[2].m_rdata[PIdx::theta][slot2_idx] = soa_1.m_rdata[PIdx::theta][src1_idx];
soa_products_data[3].m_rdata[PIdx::theta][slot3_idx] = soa_0.m_rdata[PIdx::theta][src0_idx];
soa_products_data[2].m_rdata[PIdx::phi][slot2_idx] = soa_1.m_rdata[PIdx::phi][src1_idx];
soa_products_data[3].m_rdata[PIdx::phi][slot3_idx] = soa_0.m_rdata[PIdx::phi][src0_idx];
#elif defined(WARPX_DIM_3D)
soa_products_data[2].m_rdata[PIdx::x][slot2_idx] = soa_1.m_rdata[PIdx::x][src1_idx];
soa_products_data[2].m_rdata[PIdx::y][slot2_idx] = soa_1.m_rdata[PIdx::y][src1_idx];
soa_products_data[3].m_rdata[PIdx::x][slot3_idx] = soa_0.m_rdata[PIdx::x][src0_idx];
soa_products_data[3].m_rdata[PIdx::y][slot3_idx] = soa_0.m_rdata[PIdx::y][src0_idx];
#elif defined(WARPX_DIM_XZ)
soa_products_data[2].m_rdata[PIdx::x][slot2_idx] = soa_1.m_rdata[PIdx::x][src1_idx];
soa_products_data[3].m_rdata[PIdx::x][slot3_idx] = soa_0.m_rdata[PIdx::x][src0_idx];
#endif
Thoughts?
There was a problem hiding this comment.
Maybe I don't know what is happening here, but could this be done using SmartCopy?
There was a problem hiding this comment.
@roelof-groenewald Good point, thanks for catching this.
There was a problem hiding this comment.
@dpgrote SmartCopy is being used here to create two new particles (soa_products_data[2] particle at slot2_idx based on soa_0 parent particle at src0_idx and soa_products_data[3] particle at slot3_idx based on parent particle sao_1 particle at src1_idx). However, in order to keep charge conservation during the charge exchange process the location of the two new particles is being swapped so that their locations depend on the other one's parent particle.
4f6ff4e to
39fcf1d
Compare
The previous `for (int idim = 0; idim < PIdx::w; ++idim)` loop assumed position attributes were stored at indices 0..PIdx::w-1, which is true for 1D_Z, XZ, and 3D, but NOT for RZ, RCYLINDER, and RSPHERE — there theta and phi come after `w` in PIdx. Replace with a range-based for over a `static constexpr std::array<int, N>` whose contents are selected per-dimension via `#if defined(WARPX_DIM_*)`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
7ccf3c6 to
31eaa1c
Compare
A file-scope `static constexpr std::array` has no device address, so NVCC fails with "identifier 'position_indices' is undefined in device code" when the range-based for loop tries to call begin()/end() on it. Declaring the array as a local `constexpr` inside the device lambda bakes the values into the kernel as compile-time constants and works on both CPU and GPU. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
roelof-groenewald
left a comment
There was a problem hiding this comment.
Thanks! Looks good now!
66006c9
into
BLAST-WarpX:development


Description updated by @RemiLehe
Motivation
We recently observed that combining charge exchange with the electrostatic solver (in the
developmentbranch) leads to unphysical heating (i.e. energy conservation breaks). After investigation, it seems that this is because the charge "jumps" from one macroparticle to another when a charge exchange event happens.More specifically, consider the reaction
$$A^+ + B \rightarrow A + B^+$$
between one A+ macroparticle and B macroparticle that are in the same cell but not necessarily at the exact same location. By default (in the
developmentbranch),B+is created at the position ofBandAis created at the position ofA+. But that means that the charge suddenly moves from one position to another in one timestep. When recomputing the electrostatic fields at the following timestep (with the electrostatic solver), the field configuration is thus substantially different. It seems that this provides a sudden "kick" to the particles and is what leads to a lack of energy conservation.The solution implemented in this PR is to create the
B+particle at the position ofA+(andBat the position ofA). This appears to fix energy conservation, for a uniform thermal plasma (see more details below):What this PR does
Fix: Reintroduces a dedicated
CHARGE_EXCHANGEreaction type (previously conflated withTWO_PRODUCT_REACTION) so that, upon a charge exchange event, the positions of the two product particles are swapped. This ensures that charge does not jump across space in a single timestep, restoring charge conservation.New test: Adds a test analogous to
test_2d_collisions_split_momentum_push_electrostatic(which verifies energy conservation when Coulomb scattering is combined with the electrostatic solver). A new input script is introduced that replaces Coulomb scattering with charge exchange and uses three species — neutrals, ions, and neutralizing electrons — instead of two. The existing analysis script (which checks energy conservation and energy equipartition) is reused but generalized to remove its hardcoded two-species assumption. The test is designed to fail if the position-swap fix is not applied.