Support domain deformations in elliptic solver #5575
Conversation
nilsvu
force-pushed
the
elliptic_time_dependent_maps
branch
from
1cc38e4
to
c9f6f33
Compare
src/Domain/ElementMap.cpp
Outdated
| Frame::BlockLogical, TargetFrame, Dim>> { | ||
| if constexpr (std::is_same_v<TargetFrame, Frame::Inertial>) { | ||
| if (block.is_time_dependent()) { | ||
| if (block.has_distorted_frame()) { |
There was a problem hiding this comment.
Do you actually need this case? Shouldn't the else branch work even if there's a distorted frame? I'm fairly confident the evolution code at least requires grid->inertial to always be a valid map, independent of whether or not there's a distorted frame.
There was a problem hiding this comment.
You're right 👍 removed the unnecessary case.
| @@ -70,15 +70,14 @@ target_link_libraries( | |||
| DomainBoundaryConditions | |||
There was a problem hiding this comment.
In the elliptic code do you actually evaluate the maps outside of compute items? If not, this commit, if not the previous as well could be replaced by adding a grid->inertial map into the elliptic box and using the time-dependent compute tags we use in the evolution code. Those tags don't do any extra work for "static" domains. I guess they currently require a Tags::Time to be in the box, but we could reasonably template on a bool or something to elide the time if it's not needed.
There was a problem hiding this comment.
Currently the map is only needed during initialization; in multiple actions though (domain init, subdomain init, faces & mortars). I thought about removing the ElementMap tag from the DataBox, but kept it for now so I don't have to recreate the ElementMap in these places. Could be done though I think. When we update the map parameters we have to re-evaluate the map as well though, which might be easier if the ElementMap is available. I haven't thought that part through yet.
There was a problem hiding this comment.
Alright, that sounds good. Thanks for the explanation!
| // Coordinates | ||
| *logical_coords = logical_coordinates(*mesh); | ||
| *inertial_coords = element_map->operator()(*logical_coords); | ||
| *inertial_coords = |
There was a problem hiding this comment.
Right, if you don't need the map in other places, this seems like the perfect place to just call the grid->inertial map once
I hit this limit by increasing the number of tags in the DataBox.
nilsvu
force-pushed
the
elliptic_time_dependent_maps
branch
from
33596f2
to
ba70951
Compare
This enables horizon-conforming (ellipsoidal) excision surfaces for spinning BH initial data. It will also allow for surface-fitting coordinates for BNS initial data.
nilsvu
force-pushed
the
elliptic_time_dependent_maps
branch
from
ba70951
to
a5e0ae6
Compare
|
Fixed a use-after-move clang-tidy complaint in the ElementMap constructor |
Proposed changes
This enables horizon-conforming (ellipsoidal) excision surfaces for spinning BH initial data. It will also allow for surface-fitting coordinates for BNS initial data. For reference, here's my thesis branch that had this implemented in a different way (using static deformed maps, not time dependence): develop...nilsvu:spectre:thesis
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