Use stronger log maps in BBH ID domain for high mass ratios#7283
Use stronger log maps in BBH ID domain for high mass ratios#7283iago-mendes wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Code <noreply@anthropic.com>
| "Use a logarithmically spaced radial grid in the part of Layer 1 " | ||
| "enveloping the object (requires the interior is excised)"}; | ||
| }; | ||
| struct CubeBLogMapStrength { |
There was a problem hiding this comment.
I think just name this CubeLogMapStrength as it appears in both objects. It can just be set to 1 for object A in the input file.
| @@ -543,13 +544,27 @@ Domain<3> BinaryCompactObject<UseWorldtube>::create_domain() const { | |||
| inner_sphericity_B, 1.0, use_equiangular_map_, | |||
| offset_b_optional, false, {}, object_B_radial_distribution), | |||
There was a problem hiding this comment.
Do I understand correctly that these inner spherical wedges have a hardcoded singularity position if log scaling is enabled, and this might not match the singularity position in the cube? Does this lead to mismatches in the radial element size (e.i., radial Jacobian) at the sphere-to-cube boundary? I think ideally the radial Jacobian is continuous at this boundary (not sure how important that is)
| const auto grid_distribution = RadialInterval3D{ | ||
| Identity{}, Identity{}, | ||
| Interval{-1., 1., -1., 1., | ||
| domain::CoordinateMaps::Distribution::Logarithmic, |
There was a problem hiding this comment.
Does now always enable log map in the cube, where before it was always linear? Is it better to make that an option or use the existing option so it's the same as the inner spheres?
| (cube_b_R_in - alpha * cube_b_R_out); | ||
| const double logical_r0 = | ||
| (2.0 * physical_r0 - (cube_b_R_out + cube_b_R_in)) / | ||
| (cube_b_R_out - cube_b_R_in); |
There was a problem hiding this comment.
Can you add a comment explaining this math a bit (how you compute this based on the log map strength parameter)?
nilsdeppe
left a comment
There was a problem hiding this comment.
This isn't an ID change only. This also changes tolerances in the evolution to values that enormous. I don't think we can reasonably accept this change to the whole code base. If we want to limit it to ID, I guess, but the fact that you need to reduce the accuracy to single precision is indicative of bad math cancellations that should be fixed instead. Essentially, I think the changes with local_eps in BlackLogicalCoordinates introduces a major latent bug.
Co-Authored-By: Claude Code <noreply@anthropic.com>
524c439 to
0ca2021
Compare
|
@nilsdeppe, thanks for your comment. Indeed, loosening the tolerances was only a temporary workaround and wasn't planned to be merged as shown before. I've done some more testing, and I was surprised to find that I'm able to remove most of the tolerance workaround that I was using before. I've now replaced the previous "Loosen tolerances..." commit with "Use center as roundoff scale...". I've found that using the center of AhB as a scale for With regard to the previous changes in In summary, the new approach doesn't introduce a "major latent bug" anymore. It still isn't an ID change only (as this also affects anything using the Wedge map), but it is a more careful loosening of the tolerances used when checking whether a coordinate is in the interior or not (which raises the effective epsilon by ~D in the extreme cases). |
Proposed changes
This PR adds the approach I've been able to use to produce BBH ID up to mass ratios$10^6$ . The basic idea is to move the origin of the log map in the cube around the small black hole.
Note: for this to work, I had to loosen some tolerances in the domain logic. This is what the second commit does. I haven't tried to use a more clever approach, so I'm hoping that some suggestions will come up during code review.
Here's what the domain looks like with these changes:
$q=1$ (same as before):

$q=100$ :

$q=10^6$ :

The cost scaling with mass ratio looks like the plot below (all non-spinning cases):

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."Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com",
"Co-Authored-by: Codex noreply@openai.com", or
"Co-Authored-By: GitHub Copilot CLI noreply@microsoft.com"
as the last line of the commit, depending on the agent.
Further comments