Skip to content

Conversation

@knelli2
Copy link
Contributor

@knelli2 knelli2 commented Jul 21, 2025

Proposed changes

Tenth PR in horizon finder changes. Adds a function that sets up the databox for callbacks to be run. Actual callbacks will be added in future PRs.

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

@knelli2 knelli2 added this to the AH finder improvements milestone Jul 21, 2025
@knelli2 knelli2 requested a review from wthrowe July 21, 2025 07:16

namespace ah {
template <typename HorizonMetavars, typename DbTags, typename Metavariables>
void invoke_callbacks(const gsl::not_null<db::DataBox<DbTags>*> box,
Copy link
Member

Choose a reason for hiding this comment

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

Needs documentation.

new_interpolated_vars->initialize(current_ylm.physical_size());
tmpl::for_each<ah::vars_to_interpolate_to_target<3, Fr>>(
[&](auto tag_v) {
using tag = typename decltype(tag_v)::type;
Copy link
Member

Choose a reason for hiding this comment

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

[optional] Use an explicit template argument. A few other places as well.

});

// Time deriv is zero because there weren't any previous horizons to come
// the time deriv with
Copy link
Member

Choose a reason for hiding this comment

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

"come" -> "compute", I think?

*horizon = current_strahlkorper;

// This is a hack to use ylm::time_deriv_of_strahlkorper function below
std::deque<std::pair<double, ylm::Strahlkorper<Fr>>>
Copy link
Member

Choose a reason for hiding this comment

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

Not ideal, but it should be cheap compared to the horizon-find, so probably OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah when we do a profile, we can see if it pops up

Copy link
Member

@wthrowe wthrowe 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 except for one typo. Squash.

*
* \details Before invoking the callbacks, this function
*
* 1. Restricts the final interpolated variables from the $L_\mathrm{mesh}$ use
Copy link
Member

Choose a reason for hiding this comment

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

use -> used

@knelli2
Copy link
Contributor Author

knelli2 commented Jul 22, 2025

Done!

@knelli2 knelli2 mentioned this pull request Jul 22, 2025
3 tasks
@wthrowe
Copy link
Member

wthrowe commented Jul 23, 2025

The Kokkos build had a number of odd failures (even more than usual), but none of them look plausibly related to these changes, so I'm ignoring them.

@wthrowe wthrowe merged commit a60ae40 into sxs-collaboration:develop Jul 23, 2025
23 of 24 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