-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Subcell: Add TciOnFdGrid mutator for NewtonianEuler #3199
Subcell: Add TciOnFdGrid mutator for NewtonianEuler #3199
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can squash
tests/Unit/Evolution/Systems/NewtonianEuler/Subcell/Test_TciOnFdGrid.cpp
Outdated
Show resolved
Hide resolved
min(get(get<Pressure>(dg_grid_prim_vars)))) < | ||
min_pressure_allowed or | ||
evolution::dg::subcell::persson_tci(dg_mass_density, dg_mesh, | ||
persson_exponent, 1.0e-18) or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make a constexpr double persson_tci_epsilon = 1.0e-18;
as in the other TCI?
* minimum density and pressure on both the DG and subcell mesh are above | ||
* \f$10^{-18}\f$, and runs the Persson TCI on the density and pressure on the | ||
* DG grid. The reason for applying the Persson TCI to both the density and | ||
* pressure is to flag cells at contact discontinuities. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update this to match the newer text in the other TCI
Also, can you explain why you check both grids in a function "TciOnFdGrid" ?
a50ed29
to
92d0487
Compare
I've rebased and pushed a fixup. Thanks for the reviews! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have my permission to squash (including the one additional requested change)
* present in the FD solution. | ||
* - runs the Persson TCI on the density and pressure on the DG grid. The reason | ||
* for applying the Persson TCI to both the density and pressure is to flag | ||
* cells at contact discontinuities. The Persson TCI is only works with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is only -> only
LGTM, please squash |
92d0487
to
7efffda
Compare
I've squashed. Thanks for reviewing :) |
Proposed changes
Add the troubled-cell indicator on the FD grid for NewtonianEuler.
Upgrade instructions
Code review checklist
make doc
to generate the documentation locally intoBUILD_DIR/docs/html
.Then open
index.html
.code review guide.
bugfix
ormajor new feature
if appropriate.Further comments