search

lanl / phoebus

Phifty One Ergs Blows Up A Star
BSD 3-Clause "New" or "Revised" License
32 stars 0 forks source link

Fix & refector: Incorrect v^2 calc in some pgens, and move Reducers out of Driver class. #188

Closed AstroBarker closed 9 months ago

AstroBarker commented 9 months ago

The name says it all. When I was trying to test tracers with the 2D advection problem, I eventually found that the vsq calculation was erroneous, showing the following pattern (psueudocode):

SPACELOOP2(ii, jj) { vsq += v(ii) * v(jj); }

Namely, it was either 1) missing the metric (which would collapse the sum in Minkowski or 2) should've been manually collapsed to just SPACELOOP(ii). The result was vsq > 1 and unphysical Lorentz factors. This PR fixes that by pulling in the metric (opting for the more general solution). I noticed the same pattern in the kh and linear_modes problems. kh is fixed the same, while linear_modes I replaced the loop with a 1D SPACELOOP to not fight with the existing metric for when using e.g., snake coordinates.

There's a (somewhat) unrelated refactor in this PR. I was working on adding a Parthenon-style advection regression test #187 but ran into an issue: the reducer objects for e.g., particle_resolution in the Driver class would have their destructors called after MPI/Kokkos finalize and Phoebus would exit with an error code, which caused the regression testing to exit with an error, too. So, this PR also refactors those Reducer objects as mutable params into radiation and fluid, where appropriate. I also split off the Driver creation and execution into its own scope. Not necessary with the refactor, but safer and potentially more future-proof. In a separate PR I'll pull in the regression testing (see #187 ).

Yurlungur commented 9 months ago

My comments are non-blockers, so I'm just going to merge. But may be wroth revisiting them at some point.