Include BBH+scalar field initial data code from Canuda in ET

[Cheng-Hsin Cheng]

We propose for inclusion the thorns TwoPunctures_BBHSF and NPScalars_SF from Canuda/Scalar. The codes are hosted in the UIUC branch: https://bitbucket.org/canuda/scalar/src/Canuda_SF_UIUC/

TwoPunctures_BBHSF is an initial data solver based on TwoPunctures for a black hole binary plus a cloud of massive scalar field. Namely, it solves the Hamiltonian constraint for the BBH system with back-reaction from a massive scalar field minimally-coupled to GR. Currently, it supports scalar field configurations with vanishing initial momentum and a spherically symmetric or dipolar angular profile, centered at the origin.

NPScalars_SF is an extension of the NPScalars thorn that computes the gravito-electric and gravito-magnetic fields taking into account contributions from the massive scalar field.

[Roland Haas]

@Thiago Assumpcao will review TwoPunctures_BBHSF

[Peter Diener]

NPScalars_SF does no harm. It compiled out of the box and didn’t cause any tests that passed before to fail.

[Samuel Cupp]

Leo and I had already received an email regarding TwoPunctures_BBHSF, but @Thiago Assumpcao has finished his review, so all that remains is completing the review for NPScalars_SF. Whenever @Peter Diener has an update, please let us know.

[Cheng-Hsin Cheng]

@Samuel Cupp @Peter Diener Apologies for the lack of updates on NPScalars_SF. But Helvi and Giuseppe and I were only discussing today about withdrawing it from inclusion into the next ETK release, and instead to update the original NPScalars thorn in the future to support other matter terms including scalar fields. From what I understand, Miguel thinks this would be a better decision for the code too. So we would like to retract NPScalars_SF from the next release, and Peter is off the hook from reviewing it.

Dropping NPScalars_SF will affect the parameter files in TwoPunctures_BBHSF which are using it, so we will also update the parameter files there to use the original NPScalars.

[Samuel Cupp]

Ok. Thank you for the update. If you don’t mind, can you also update the PR on the manifest? I believe there are also some commits to merge in as well. If you are ready, you can also merge TwoPunctures_BBHSF into master and remove the REPO_BRANCH line from the PR.

[Cheng-Hsin Cheng]

Yes, I will remove NPScalars_SF from the PR on the manifest. About the REPO_BRANCH, is it possible for us to use a separate branch from master for our proposal for the next release?

[Samuel Cupp]

That should be ok, but I’d like a maintainer to weigh in. When we make the ET_2023_05 branch, we would just have to branch it off of that instead of master. @Roland Haas @Steven R. Brandt any issues with this?

[Roland Haas]

uh, not quite sure I follow. For the ET owned repo’s branch (I guess manifest is the repo) branching off of master is preferred. The reason being that “master” is the branch that actually gets tested on the cluster and that people who want the bleeding edge code download.
For branches in Canuda’s home org: your choice really. As long as the master branch of einsteintoolkit/manifest/einsteintoolkit.th points to the the proposed branch in Canuda (after the does no harm) then this is fine (eg a couple contributions use “main” instead of “master”). There is a slight advantage if the proposed branch is the default branch since that is the one GetComponents will chose if not branch name is given (and the vanilla “development” thornlist has no REPO_BRANCH statements). But that is very minor and really only exploitable if the default branch is the branch development happens on (and not eg the last stable release one).

Note for release manager: having REPO_BRANCH in the development thornlist will likely require some changes to the scripts (and script fragments) used to create the release thornlist (since it assumes that it can just add REPO_BRANCH and does not have to modify existing REPO_BRANCH statements).

The branch name in outside repos is not under control anyway and with GitHub advertising against “master” there is a proliferation anyway (“main”, “devel”, “development” seem to be popular).

[Roland Haas]

With the inclusion progressing, would it be possible to add push hooks so that commit emails are sent to the ET commits mailing list? See here: https://docs.einsteintoolkit.org/et-docs/Services#git

[Giuseppe Ficarra]

Hi all, after deciding to withdraw NPScalars_SF from the proposal, I have removed any reference of it from both test and example parameter files of TwoPunctures_BBHSF. @Thiago Assumpcao do you want to have a look at this? Essentially I removed it completely from the test parameter file (and updated the output) and I replaced it with the original NPScalars (which is already in the ETK) in the example parameter files. Please use the new branch for the proposal https://bitbucket.org/canuda/scalar/src/proposal_ET_2023_05/ to checkout the updated version of the code.

[Thiago Assumpcao]

Hi all, I have reviewed Giuseppe’s most recent changes to TwoPunctures_BBHSF. The code has very good documentation, is well commented and has tests together with their expect output.

‌

[Roland Haas]

Right now the code for TwoPunctures_BBHSH in the ET manifest thornlist is from branch proposal_ET_2023_05 in Canuda. While ok in principle it is a bit strange to not have this eg in either development (since it saw changes) or master (which for Canuda seems to be the “trusted” branch). If included in the ET the code is expected to be “good' and what the authors use themselves.

Eg for sure for the eventual release the changes must be in an ET_2023_05 branch which normally should be a child of the trusted branch (master in this case).

Since the ET manifests master branch is not the “trusted good” but the “bleeding edge” branch (for the ET), one should be able to pick up bug fixes on ET released thorns in it without changing branches. With proposal_ET_2023_05 does seems unlikely since I would expect bug fixes to show up either in development or masterof Scalar, wouldn't they?

[Cheng-Hsin Cheng]

Hi all, after discussing with the other authors and maintainers of Canuda/Scalar, we decided to merge TwoPunctures_BBHSF from the branch proposal_ET_2023_05 into master. So any updates or bug fixes for the new thorn will happen on master, and proposal_ET_2023_05 would not be maintained.

@Roland Haas could we update the manifest thornlist to use the master branch for Scalar? Should we make another pull request for the change, or can the release team just make the changes?

[Samuel Cupp]

I can change the manifest to point to master.

[Samuel Cupp]

@Roland Haas Do you mind if I also move the canuda-lean down with the other canuda repos? Its a bit weird that there are several repos between them despite the header

# Canuda thorns
#===============

since the intervening repos are clearly not canuda.

[Samuel Cupp]

These new features are now included as part of the Toolkit in the ET_2023_05 release.