Add ResizeAndComputePrimitives to ValenciaDivClean #3289
Conversation
| #include "Evolution/DgSubcell/Tags/Mesh.hpp" | ||
| #include "Evolution/Systems/GrMhd/ValenciaDivClean/Tags.hpp" | ||
| #include "PointwiseFunctions/GeneralRelativity/TagsDeclarations.hpp" | ||
| #include "PointwiseFunctions/Hydro/EquationsOfState/EquationOfState.hpp" |
|
|
||
| template <size_t ThermodynamicDim> | ||
| static void apply( | ||
| gsl::not_null<Variables<hydro::grmhd_tags<DataVector>>*> prim_vars, |
tests/Unit/Evolution/Systems/GrMhd/ValenciaDivClean/Subcell/Test_ResizeAndComputePrimitives.cpp
Show resolved
Hide resolved
| // Use a small range of random values since we need recovery to succeed and we | ||
| // also reconstruct to the DG grid from the FD grid and need to maintain a | ||
| // somewhat reasonable state on both grids. | ||
| std::uniform_real_distribution<> dist(0.01, 0.010005); |
There was a problem hiding this comment.
is this limit because you messed up the Lorentz factor?
There was a problem hiding this comment.
no, it's because reconstructing random numbers leads to wild oscillations
There was a problem hiding this comment.
how about a narrow range around 0.5? that at least would have caught your incorrect lorentz factor
There was a problem hiding this comment.
seems to work so far. I'll squash it in once @fmahebert has looked at the fixups
| namespace grmhd::ValenciaDivClean::subcell { | ||
| /*! | ||
| * \brief Mutator that resizes the primitive variables to have the size of the | ||
| * active mesh and then computes the primitive variables on the active mesh. |
There was a problem hiding this comment.
the implementation only does these things if active_grid == Subcell
| const size_t num_grid_points = | ||
| (active_grid == evolution::dg::subcell::ActiveGrid::Dg ? dg_mesh | ||
| : subcell_mesh) | ||
| .number_of_grid_points(); |
There was a problem hiding this comment.
Can you compute this inside the if statement?
| * If the active grid is DG (we are switching from subcell back to DG) then this | ||
| * mutator computes the primitive variables on the active grid. We reconstruct | ||
| * the pressure to the DG grid to give a high-order initial guess for the | ||
| * primitive recovery. It would be nice if we can avoid this reconstruction when |
There was a problem hiding this comment.
I think it'd help to describe this clearly as a possible optimization? E.g., "A possible future optimization would be to avoid this reconstruction when...".
| subcell_spatial_metric.get(i, i) = 1.0 + 0.01 * i; | ||
| } | ||
| get(get<hydro::Tags::LorentzFactor<DataVector>>(prim_vars)) = | ||
| sqrt(1.0 + get(dot_product(spatial_velocity, spatial_velocity, |
There was a problem hiding this comment.
Should this compute 1 / sqrt(1 - v2) ?
|
I've pushed a fixup. Thanks for the reviews :) |
kidder
left a comment
There was a problem hiding this comment.
you have my permission to squash
9f74ea0 to
55c42bd
Compare
|
I've rebased and squashed. Thanks for the reviews :) |
Proposed changes
Add mutator used to resize and compute the prims on the DG grid when switching from DG back to FD
Upgrade instructions
Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixormajor new featureif appropriate.Further comments