Maybe I am confused, but isn't area_element precisely the curved space area element?

yes, well spotted

Nitpicking, but I think better is "The area element, i.e. the determinant of the induced Jacobian, on a surface."

typo area

tl;dr I suggest this:

• Keep this tag unchanged (no rename, no change in what it stores).
• Include the metric determinant in your area_element functions so they actually integrate to the area in curved space.

Details: I think in curved space we should make the distinction between the surface Jacobian determinant J and the area element \sqrt{gamma} J. The area element should actually integrate to the area, hence it should include the metric determinant. Arguably the surface Jacobian alone (without the metric determinant) isn't much use, but you probably don't want to go through the code that uses the Tags::DetSurfaceJacobian and make sure it uses the metric determinant properly. Therefore, just keep this tag unchanged. But in your area_element functions include the metric determinant, and check in the test that you get the horizon area of Kerr by just integrating over the area element.

I don't have a strong opinion but this is then inconsistent with area_element in StrahlKorperGr.cpp from what I can tell.

In the StrahlkorperGr test here https://github.com/sxs-collaboration/spectre/blob/develop/tests/Unit/ApparentHorizons/Test_StrahlkorperGr.cpp#L417 the integral over the area element is the area, without an additional metric determinant. It follows Appendix D in https://arxiv.org/abs/gr-qc/9606010, which is explained in a bit more detail in Appendix C in the Baumgarte/Shapiro book. That's what we should do for this area_element function as well. I think multiplying by the spatial metric determinant is enough.

ah right, that makes sense. I changed it so

Use /// @{ to group the dox

You can also use CoordinateMaps::Composition to get the combined logical -> grid -> kerr deformed map. Then you don't need to concatenate Jacobians yourself etc. You can get the ElementLogical -> BlockLogical map from ElementToBlockLogicalMap.hpp, then compose with block.stationary_map() and the Kerr conforming map.

I am not sure how this is supposed to work. The problem is that the Kerr conforming map effectively goes from the intertial to the inertial frame, so I cannot put it directly into Composition because that will not be instantiated.

So I have to somehow concatenate block.stationary_map() and the Kerr conforming map into a single CoordinateMap before that goes from BlockLogical ->Inertial, presumably using make_coordinate_map. But block.stationary_map() is of type CoordinateMapBase and make_coordinate_map needs the dynamic type at compile time as far as I can tell, so Im not sure how to do this.

Here's an example:

        domain::CoordinateMaps::Composition coord_map{
            domain::element_to_block_logical_map(element_id),
            current_block.stationary_map().get_to_grid_frame(),
            std::make_unique<
                CoordinateMap<Frame::Grid, Frame::Inertial,
                              domain::CoordinateMaps::KerrHorizonConforming>>(
                kerr_schild_map)};
        const auto inertial_coords = coord_map(face_logical_coords);
        const auto inv_jacobian = coord_map.inv_jacobian(face_logical_coords);

I tested that this compiles.

I see, so we get the map to the grid frame so all the frames are different. Do you think it would be a good idea to put the Composition implementation in the header file so it is possible to also have maps that map to the same frame in there? Currently this is not possible because the instantiation would not be there.

This also threw up a bug #4851 which needs to merged before this test can pass.

Update dox with metric determinant

I think the different maps and frames are a bit confusing here (e.g. map is named grid but coords are named inertial, then there's another Inertial->Inertial map). Please clear this up a bit, or better yet use a Composition map that combines ElementLogical->BlockLogical->Wedge->HorizonConforming into one map (see my comment above #4743 (comment)).