Port extraction class to ParticleInterpolator by tamaraevst · Pull Request #174 · GRTLCollaboration/GRTeclyn

tamaraevst · 2026-04-13T22:01:25Z

This is the first draft for porting all of the Extraction* classes to GRTeclyn and addressing issue #16. Key features:

The class structure of all extraction files being ported here inherit everything from GRChombo. Most importantly, the inheritance of SphericalExtraction from SurfaceExtraction is the same.
All of the extraction is fully integrated with the ParticleInterpolator.
I have tested the GW extraction on a $$q=1$$ non-spinning BBH head-on collision using initial separation of $$d = 10M$$ and no boost. Comparisons across different resolutions, extraction radii within GRTeclyn and medium and high resolutions between GRChombo and GRTeclyn are attached below. In summary, within GRTeclyn I get convergence of around 2nd order on the radiated energy. When comparing to GRChombo, I get closer results with GRTeclyn at medium and high resolutions. My resolutions correspond to 1/64, 1/96 and 1/128 on the finest levels for low, med and high resolutions, respectively. I forgot to put y-labels, but I am showing the (22)-mode only. The agreement/comparison for the (20)-mode looks qualitatively similar. A sample parameter file (at medium resolution) I used is also attached in file params.txt.
The test for SphericalExtraction has also been ported. We treat spherical harmonics as state variables there. This choice was deliberate as the above was an "indirect" tests for derived-like variables.

I have a couple of comments in regards to the current structure, which I find sub-optimal:

SphericalExtraction parameters are stored in SimulationParametersBase. Because our ParticleInterpolator is templated over the number of components we are interpolating over, SphericalExtraction needs to inherit the exact same template. Now, this means in SimulationParametersBase we need to instate this number of components. Currently, it is hard-coded as a number. One would have hoped that it would be possible to use BHAMR<BinaryBHLevel::num_punctures>::particle_num_components but this results in circular includes. The only improvement I could come up with is storing particle_num_components elsewhere (e.g. in some extraction specific struct).
I had to move the enum for StateType to a separate file, as I was again hitting circular includes. Was there a reason for defining it globally inside GRAMRLevel?

github-actions · 2026-04-13T22:01:40Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

mirenradia

Thanks for the PR :) I have some minor comments but I think they should mostly be straightforward to fix.

KAClough · 2026-04-20T14:28:52Z

Hi, I added some very minor comments, so I think once you have addressed Miren's changes this is good to go! I do think we need a simpler version of the spherical extraction class, but we can either amend these classes later or maybe just add an additional class called SimpleSphericalExtraction that does a single scalar and integrates it. The main thing is that this seems to work, so we can use it to do some more testing of inspirals.

github-actions · 2026-05-01T21:35:36Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

github-actions · 2026-05-04T20:37:09Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

github-actions · 2026-05-04T20:57:31Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

github-actions · 2026-05-04T21:05:57Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

github-actions · 2026-05-04T22:48:09Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

tamaraevst · 2026-05-05T12:10:07Z

Hi @mirenradia @KAClough I have addressed you comments. Please have a look and let me know. I replied directly to your comments in cases where it was not a straightforward fix. I have linter issues, but will resolve them once you are happy with the rest of the changes.

mirenradia · 2026-05-06T08:32:19Z

Hi @mirenradia @KAClough I have addressed you comments. Please have a look and let me know. I replied directly to your comments in cases where it was not a straightforward fix. I have linter issues, but will resolve them once you are happy with the rest of the changes.

@tamaraevst, I won't be able to re-review this until next week if that influences whether you want to start fixing the linter issues rather than waiting for me.

github-actions · 2026-05-22T14:11:01Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

github-actions · 2026-05-23T03:05:00Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

github-actions · 2026-05-26T21:01:05Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

github-actions · 2026-05-26T21:13:38Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

* Ported CustomExtraction * Ported SurfaceExtraction * Ported SphericalExtraction * Ported WeylExtraction

* Updated BinaryBH example with GW extraction * Some rewrite of Extraction* classes * Added a new file to store StateType * Ported SphericalExtraction test * Remove old files

* Restructure SurfaceExtraction class * Rename CustomExtraction to LineExtraction and make it more general * Some minor renaming of variables and minor commenting

* Renamed params_t structs in Spherical and Surface Extractions * Moved params_t outside of the classes in Spherical and Surface Extractions (to avoid magic numbers in ExtractionTagger and SimulationParametersBase) * Tidied up "include" order in tests * Removed redundant state_index arg in interp() function of the ParticleInterpolator

* Moved extraction parameters to separate files * Added some error checks/messages

* Fix arena init size to 0 in the input file * Remove some unenecessary debugging statements * Fix some linter issues

* Switched to using RoundOffLo() and RoundOffHi() instead of ProbLo and ProbHi * Added a check when the interpolation point lies on the symmetry axis and interpolation stencil needing adjustments

github-actions · 2026-06-22T17:21:59Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

tamaraevst · 2026-06-22T17:33:07Z

Hi all, I have now resolved the two issues concerning the gitlab failing runs.

First, I changed the CI for gitlab in the form that Miren suggested here.

Second, the gitlab run was failing when running the params_test.txt file with the BinaryBH example (where extraction was switched on). With the default extraction paramaters enforced in the file, there were some points being extracted very close to the axis of symmetry. Because we use round in Langrange.hpp to estimate the centering of the stencil, this pushed the stencil out of bounds. I therefore put a work-around for such a case here.

I also thought that it would be appropriate to use e.g. RoundOffLo() from Geometry to check when the particle is off-domain (see here). This change has also been implemeted in this PR.

KAClough · 2026-06-22T17:42:55Z

Just to note that I tried running this on LUMI-G and with multiple GPU nodes I get a segfault (but not with a single node). Have you tried this @tamaraevst or @mirenradia ?

tamaraevst · 2026-06-22T17:45:58Z

Just to note that I tried running this on LUMI-G and with multiple GPU nodes I get a segfault (but not with a single node). Have you tried this @tamaraevst or @mirenradia ?

Nope, I only have access to 1 GPU. Do you have a backtrace/log file?

KAClough · 2026-06-22T17:58:38Z

Backtrace_32.txt

It only seems to happen when I use 8 nodes, maybe it is a LUMI problem specifically. This is the backtrace - I only get the ones for 32-63.

tamaraevst · 2026-06-22T18:43:28Z

Backtrace_32.txt

It only seems to happen when I use 8 nodes, maybe it is a LUMI problem specifically. This is the backtrace - I only get the ones for 32-63.

Looks like this is coming from populate_from_query(). I wonder whether this has to do with MFIter at lev=0 not existing on certain ranks.

github-actions · 2026-06-23T17:20:47Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

github-actions · 2026-06-26T14:16:16Z

This PR modifies the following files which are ignored by .lint-ignore:

Source/BlackHoles/PunctureTracker.impl.hpp
Source/ParticleInterpolator/ParticleInterpolator.impl.hpp
Source/ParticleInterpolator/SurfaceExtraction.impl.hpp

Please consider removing the corresponding patterns from .lint-ignore so that these files can be linted.

tamaraevst requested a review from KAClough April 13, 2026 22:01

tamaraevst self-assigned this Apr 13, 2026

tamaraevst added this to GRTeclyn Apr 13, 2026

tamaraevst added enhancement New feature or request high priority Important thing to resolve labels Apr 13, 2026

mirenradia reviewed Apr 15, 2026

View reviewed changes

KAClough reviewed Apr 20, 2026

View reviewed changes

Comment thread Source/BlackHoles/BHAMR.hpp Outdated

Comment thread Source/GRTeclynCore/StateTypes.hpp

Comment thread Source/ParticleInterpolator/LineExtraction.hpp

Comment thread Source/BlackHoles/BHAMR.hpp Outdated

julianakwan moved this to In Progress in GRTeclyn Apr 28, 2026

tamaraevst force-pushed the feature/extraction branch from 166db22 to deca1dd Compare May 4, 2026 21:05

mirenradia changed the title ~~Porting of various extraction classes~~ Port extraction class to ParticleInterpolator May 12, 2026

mirenradia reviewed May 27, 2026

View reviewed changes

Comment thread Examples/BinaryBH/BinaryBHLevel.cpp Outdated

mirenradia reviewed May 27, 2026

View reviewed changes

Comment thread Examples/BinaryBH/Main_BinaryBH.cpp Outdated

mirenradia reviewed May 27, 2026

View reviewed changes

Comment thread Examples/BinaryBH/params_new.txt Outdated

mirenradia reviewed May 27, 2026

View reviewed changes

Comment thread Examples/BinaryBH/BinaryBHLevel.cpp Outdated

mirenradia reviewed May 27, 2026

View reviewed changes

Comment thread Source/ParticleInterpolator/Make.package Outdated

mirenradia reviewed May 27, 2026

View reviewed changes

Comment thread Source/ParticleInterpolator/ParticleInterpolator.impl.hpp Outdated

mirenradia reviewed May 27, 2026

View reviewed changes

Comment thread Source/ParticleInterpolator/SphericalExtraction.hpp

tamaraevst added 19 commits June 22, 2026 13:15

Port Extraction files to be compatible with ParticleInterpolator

ac5c3d9

* Ported CustomExtraction * Ported SurfaceExtraction * Ported SphericalExtraction * Ported WeylExtraction

Minor refactoring in SurfaceExtraction

b497d44

Update BinaryBH example and add tests

31dd0a8

* Updated BinaryBH example with GW extraction * Some rewrite of Extraction* classes * Added a new file to store StateType * Ported SphericalExtraction test * Remove old files

Address some review comments

8e923a8

* Restructure SurfaceExtraction class * Rename CustomExtraction to LineExtraction and make it more general * Some minor renaming of variables and minor commenting

Fix copy right headers

d20bf94

Enable forward declaration of ParticleInterpolator in SurfaceExtraction

e71fda3

Minor refactor of Extraction* classes

27e4d4c

* Moved extraction parameters to separate files * Added some error checks/messages

Fix some linter issues

9119f64

Address more linter complaints

d199677

Address yet more linter errors

69d1b3a

Address remaining review comments

b7c4ddc

Refactor BHAMR class

bc9deca

Changes to SurfaceExtraction and CI

fac6ede

Address linter issues and add some debugging lines

f7336fe

Fix more linter issues

648cbc6

Adjust SphercialExtractionTest

8c8b3bf

* Fix arena init size to 0 in the input file * Remove some unenecessary debugging statements * Fix some linter issues

Improvements to ParticleInterpolator

9d30607

* Switched to using RoundOffLo() and RoundOffHi() instead of ProbLo and ProbHi * Added a check when the interpolation point lies on the symmetry axis and interpolation stencil needing adjustments

Fix linter remaining issues

9718c11

tamaraevst force-pushed the feature/extraction branch from de44b6a to 9718c11 Compare June 22, 2026 17:21

Minor change to populate_from_query

ee31b68

Minor refactor to ParticleInterpolator's populate from query function

8af7505

Uh oh!

Conversation

tamaraevst commented Apr 13, 2026

Uh oh!

github-actions Bot commented Apr 13, 2026

Uh oh!

mirenradia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KAClough commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 1, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

github-actions Bot commented May 4, 2026

Uh oh!

tamaraevst commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mirenradia commented May 6, 2026

Uh oh!

github-actions Bot commented May 22, 2026

Uh oh!

github-actions Bot commented May 23, 2026

Uh oh!

github-actions Bot commented May 26, 2026

Uh oh!

github-actions Bot commented May 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 22, 2026

Uh oh!

tamaraevst commented Jun 22, 2026

Uh oh!

KAClough commented Jun 22, 2026

Uh oh!

tamaraevst commented Jun 22, 2026

Uh oh!

KAClough commented Jun 22, 2026

Uh oh!

tamaraevst commented Jun 22, 2026

Uh oh!

github-actions Bot commented Jun 23, 2026

Uh oh!

github-actions Bot commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

KAClough commented Apr 20, 2026 •

edited

Loading

tamaraevst commented May 5, 2026 •

edited

Loading