Rotate angles to zero for binary collisions#6841
Rotate angles to zero for binary collisions#6841dpgrote wants to merge 7 commits intoBLAST-WarpX:developmentfrom
Conversation
| * momentum of the macroparticles to theta = 0. | ||
| * (This is technically only valid if we use only the m=0 azimuthal mode in the simulation; | ||
| * there is a corresponding assert statement at initialization.) */ | ||
| mypc->RotateParticleAnglesByTheta(-1.); |
There was a problem hiding this comment.
This is maybe a nuance, but I find this function name confusing. It reads as if it is rotating the particle angles by theta, but what it actually does is rotate the particle velocity vectors from physical space to mapped space. When passing +1, it does the inverse of that and converts the velocity vector from mapped space back to physical space.
Here is a suggested alternative to consider:
mypc->ConvertVelocityFromPhysicalToMapped(/*inverse=*/false);
To go from mapped back to physical, set the function argument to true.
There was a problem hiding this comment.
I changed the name to MapMomentumToXAxis. Better?
| { | ||
|
|
||
| #if defined(WARPX_DIM_RZ) || defined(WARPX_DIM_RCYLINDER) || defined(WARPX_DIM_RSPHERE) | ||
| /* In RZ and RCYLINDER geometry, macroparticles can collide with other macroparticles |
There was a problem hiding this comment.
I think that all collision types, not just binary ones, get dispatched through this function. Is this rotation valid for those as well, for example the new PulsedDecay one?
There was a problem hiding this comment.
There was a problem hiding this comment.
I think the only other ones we need to worry about are BackgroundMCC and BackgroundStopping. To me, it looks like those processes only depend on the energy of the selected particles, so rotating, computing the collisions with the background, then rotating back should be equivalent.
There was a problem hiding this comment.
I did consider this issue. In the nonbinary collisions, it is a single particle interacting with a background. By construction, that background should be cylindrically or spherically symmetric so the rotation should not matter, except perhaps producing a different random distribution. Perhaps to avoid any issues, I can add checks so that only species involved in the binary collision are rotated. This would also avoid rotating species not involved in any collisions.
Currently, there are no CI tests of those cases. There are only two tests with collisions and RZ, the one weak test with Coulomb collisions, and a nuclear fusion test.
There was a problem hiding this comment.
@dpgrote This may require further discussion with those more familiar with the non-binary scattering collision methods, but I wouldn't recommend applying this to only species involved in binary collisions. For one thing, it is very likely that species involved in non-binary collisions are involved in binary collisions as well.
I think for all collision types doing the scattering with particles in the mapped frame is the right thing to do. It may be worth checking how particles created in inelastic events are initialized. They should be initialized at zero angle to be consistent.
In binary collision in radial geometries, the particle momenta are rotated to a common angle before the collisions so that the particles are spatially local (rather than spread out in angle). Currently, this rotation is done within each of the separate collision kernels. This PR pulls this rotation out of those kernels, rotating all particles to the common angle theta = 0 before any collisions and rotating back afterward. This offers several advantages, reducing code duplication, and reducing effort (since species that participate in multiple collisions have their momenta adjusted only once instead of in each collision).
A side note, the CI test
test_rz_collisionis a fairly weak test since it fully passes whether or not the rotation is done. A suggestion is to reduce the tolerance to 1.e-35. Without the rotation, the error in the momentum is ~1.e-22, below the current tolerance of 1.e-15.