Conversation
Added CapyrX thorns configuration to spacetimex.th
|
Ticket is here |
chcheng3
left a comment
chcheng3
left a comment
There was a problem hiding this comment.
This PR looks good to me, I only have a few minor suggestions. I also appreciate that besides adding Multipatch support, the PR also makes the kernel in NewRadX_Apply easier to read by providing inline functions for computing spatial derivatives.
There was a problem hiding this comment.
Is there a use case where one would want to steer the parameter apply_inner_boundary?
| SpacetimeX/Weyl | ||
| SpacetimeX/WeylScal4 | ||
| SpacetimeX/Z4c | ||
|
|
There was a problem hiding this comment.
Can this new line be removed?
| const auto dc_dz = vJ_dc_dz(p.I); | ||
|
|
||
| // Global derivatives | ||
| const auto dgf_dx = dgf_db * db_dx + dgf_dc * dc_dx + da_dx * dgf_da; |
There was a problem hiding this comment.
This is purely cosmetic, but I would find it easier to read if the individual terms are ordered as
dgf_dx = dgf_da * da_dx + dgf_db * db_dx + dgf_dc * dc_dx
Similar comment for lines 260, 261, and 315-317.
There was a problem hiding this comment.
Can you add descriptions for the new arguments introduced for the Multipatch version of NewRadX_Apply? Namely, I would like to know the what the quantities vcoordx, vJ_da_dx, etc represent. Where would they be provided? Would a thorn implementing a Multipatch infrastructure provide them, e.g. CapyrX?
2 participants
This PR adds Multipatch support to NewRadX.
This support is done in a way that no specific multipatch driver is required, i.e., it does not require CapyrX to function. It also assumes no particular patch structure or that a special radial coordinate is available. The new functionality is provided via an overloaded function of the original, and all features are opt-in.
A new parameter has been introduced, which allows one to use the BC in inner boundaries, if the patch system contains them.