Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code to get common horizon coefs in ringdown distorted frame #6065

Conversation

geoffrey4444
Copy link
Contributor

Proposed changes

This PR is the first in a set that will enable transitioning from merger to ringdown in binary black hole simulations. This PR adds a C++ function that reads in common horizon coefficients from a file and transforms them to the ringdown distorted frame. A future PR will add a pybinding to this function and use that binding in an update to the Ringdown pipeline.

Upgrade instructions

Code review checklist

  • The code is documented and the documentation renders correctly. Run
    make doc to generate the documentation locally into BUILD_DIR/docs/html.
    Then open index.html.
  • The code follows the stylistic and code quality guidelines listed in the
    code review guide.
  • The PR lists upgrade instructions and is labeled bugfix or
    new feature if appropriate.

Further comments

@geoffrey4444
Copy link
Contributor Author

I will fix the clang-tidy failures and look into why clang is happy but gcc isn't

@geoffrey4444 geoffrey4444 force-pushed the StrhalkorperCoefsInRingdownDistortedFrame branch 4 times, most recently from 7c175c8 to 3e46eee Compare June 5, 2024 18:56
@geoffrey4444
Copy link
Contributor Author

Clang-tidy is fixed, and the gcc test failures are fixed. The test that did fail is unrelated to this PR

Comment on lines 39 to 40
const Matrix& coefs_for_times =
ahc_h5_file.get<h5::Dat>(surface_subfile_name).get_data();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to avoid reading all coefficients in, use get_data_subset instead.

  const Matrix ahc_times = ahc_h5_file.get<h5::Dat>(
          surface_subfile_name).get_data_subset({0}, 0, ahc_inertial_h5.size());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

except I want the last requested_number_of_times_from_end , not the first requested_number_of_times_from_end

src/Evolution/Ringdown/CMakeLists.txt Show resolved Hide resolved
Comment on lines +53 to +60
const auto kerr_horizon_radius = get(gr::Solutions::kerr_horizon_radius(
::ylm::Spherepack(l_max, m_max).theta_phi_points(), 1.0,
{{0.0, 0.0, 0.8}}));
const auto expected_strahlkorper = ylm::Strahlkorper<Frame::Grid>(
l_max, m_max, kerr_horizon_radius, expected_center);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is just supposed to be a Schwarzschild horizon of radius 1, you could do

const ylm::Strahlkorper<Frame::Grid> expected_strahlkorper{
    l_max, 1.0, {0.0, 0.0, 0.0};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deliberately wanted it to be a Kerr horizon with spin 0.8, so some coefs are nontrivial

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh oops I misread this. The 8 blended in with all the 0s

@knelli2 knelli2 added this to the BBH evolve through ringdown milestone Jun 11, 2024
Copy link
Contributor

@knelli2 knelli2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Squash

@geoffrey4444 geoffrey4444 force-pushed the StrhalkorperCoefsInRingdownDistortedFrame branch from 46d1e08 to 5ef8294 Compare June 12, 2024 13:06
@geoffrey4444
Copy link
Contributor Author

Test failures look to me unrelated to this PR

@knelli2
Copy link
Contributor

knelli2 commented Jun 12, 2024

Yeah they do look unrelated, but I restarted them anyways. If there are still timeouts, I'll just merge this.

@knelli2 knelli2 merged commit 64e812a into sxs-collaboration:develop Jun 12, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants