Add a reflective boundary condition #5624
Conversation
| * | ||
| * All primitive variables at the boundary are copied into ghost zone except : | ||
| * | ||
| * - If \f$n_iv^i \leq 0\f$ where \f$v^i\f$ is spatial velocity and \f$n_i\f$ |
There was a problem hiding this comment.
As far as I understood the Shiokawa paper, this boundary condition flips the normal component of velocity and the whole magnetic field regardless of whether n_iv^i > 0 or n_i B^i > 0. Here it looks like a slight variation of the soft BC in that it does not invert normal velocity when flow is pointing inward. I'm not sure how this affects the quality of simulation along the polar boundary, but if that works okay, it should be mentioned here that this is not exactly same as the one presented in Shiokawa paper.
There was a problem hiding this comment.
Also, the same treatment of normal magnetic field based on the criteria n_i B^i > 0 seems not so trivial to me. Have you tried always inverting the normal component of B field?
There was a problem hiding this comment.
I added an option to reflect both outgoing and incoming normal components of spatial velocity and magnetic field.
jyoo1042
force-pushed
the
BC_PR
branch
2 times, most recently
from
6c9ba04
to
cb0d5f4
Compare
jyoo1042
force-pushed
the
BC_PR
branch
2 times, most recently
from
f1b6669
to
82d6c7f
Compare
jyoo1042
force-pushed
the
BC_PR
branch
5 times, most recently
from
f0d33bd
to
08db55c
Compare
src/Evolution/Systems/GrMhd/ValenciaDivClean/BoundaryConditions/Reflective.cpp
Outdated
Show resolved
Hide resolved
src/Evolution/Systems/GrMhd/ValenciaDivClean/BoundaryConditions/Reflective.cpp
Outdated
Show resolved
Hide resolved
| if (direction.sign() > 0.0) { | ||
| get<LorentzFactorTimesSpatialVelocity>(outermost_prim_vars).get(i) = | ||
| get(get_boundary_val(interior_lorentz_factor)) * | ||
| min(normal_spatial_velocity_at_boundary, |
There was a problem hiding this comment.
I don't understand why there's this min call
tests/Unit/Evolution/Systems/GrMhd/ValenciaDivClean/BoundaryConditions/Test_Reflective.cpp
Show resolved
Hide resolved
tests/Unit/Evolution/Systems/GrMhd/ValenciaDivClean/BoundaryConditions/Test_Reflective.cpp
Outdated
Show resolved
Hide resolved
tests/Unit/Evolution/Systems/GrMhd/ValenciaDivClean/BoundaryConditions/Test_Reflective.cpp
Outdated
Show resolved
Hide resolved
tests/Unit/Evolution/Systems/GrMhd/ValenciaDivClean/BoundaryConditions/Test_Reflective.cpp
Outdated
Show resolved
Hide resolved
| @@ -180,6 +180,7 @@ void test_boundary_condition_with_python_impl( | |||
| const BoundaryCondition& boundary_condition, | |||
| const Index<FaceDim>& face_points, | |||
| const db::DataBox<DbTagsList>& box_of_volume_data, | |||
| std::optional<const bool> member_bool, | |||
There was a problem hiding this comment.
If I'm understanding correctly, this is literally the point of the ExtraTagsForPythonFromDataBox. Why can't you use that?
jyoo1042
force-pushed
the
BC_PR
branch
2 times, most recently
from
cc85f66
to
d7b59f2
Compare
|
@nilsdeppe addressed the comments. Thanks! |
jyoo1042
force-pushed
the
BC_PR
branch
2 times, most recently
from
141bfe0
to
a43512f
Compare
|
@nilsdeppe addressed the last comment. |
| @@ -37,10 +37,15 @@ void test_stuffs(const bool reflect_both) { | |||
| make_not_null(&gen), used_for_size); | |||
| const auto sqrt_det_spatial_metric = determinant(spatial_metric); | |||
|
|
|||
| struct RandomBool : db::SimpleTag { | |||
There was a problem hiding this comment.
Maybe name this ReflectBoth instead of RandomBool? "random" means something in CS, and it does not appear the value of this bool is randomly generated
Proposed changes
Upgrade instructions
Code review checklist
make docto generate the documentation locally intoBUILD_DIR/docs/html.Then open
index.html.code review guide.
bugfixornew featureif appropriate.Further comments