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

Update ReduceCceWorldtube with new features #6398

Merged
merged 3 commits into from
Dec 14, 2024

Conversation

knelli2
Copy link
Contributor

@knelli2 knelli2 commented Dec 5, 2024

Proposed changes

The worldtube data can now be metric modal/nodal or bondi modal/nodal and the executable can temporally combine H5 files before transforming the worldtube data into bondi modal form.

Upgrade instructions

There are two new options for the ReduceCceWorldtube executable. They are InputDataFormat: and ExtractionRadius:. Also the InputH5File option can now be either a single filename or a list of filenames to combine temporally.

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

Depends on and includes #6396, #6397.

@knelli2 knelli2 added in progress Don't review, used for sharing code and getting feedback dependent Needs a different PR to be merged in first labels Dec 5, 2024
@knelli2 knelli2 requested a review from nilsdeppe December 5, 2024 08:29
@knelli2 knelli2 force-pushed the reduce_cce_combine_h5 branch 2 times, most recently from bf2c7bf to d814ef7 Compare December 10, 2024 01:27
@knelli2 knelli2 removed the in progress Don't review, used for sharing code and getting feedback label Dec 10, 2024
@knelli2 knelli2 force-pushed the reduce_cce_combine_h5 branch 2 times, most recently from 4d57b86 to 5327598 Compare December 10, 2024 04:46
src/Executables/ReduceCceWorldtube/ReduceCceWorldtube.cpp Outdated Show resolved Hide resolved
src/Executables/ReduceCceWorldtube/ReduceCceWorldtube.cpp Outdated Show resolved Hide resolved
src/Executables/ReduceCceWorldtube/ReduceCceWorldtube.cpp Outdated Show resolved Hide resolved
src/Executables/ReduceCceWorldtube/ReduceCceWorldtube.cpp Outdated Show resolved Hide resolved
src/Executables/ReduceCceWorldtube/ReduceCceWorldtube.cpp Outdated Show resolved Hide resolved
tests/Unit/Executables/Test_ReduceCceWorldtube.cpp Outdated Show resolved Hide resolved
docs/Tutorials/CCE.md Show resolved Hide resolved
docs/Tutorials/CCE.md Outdated Show resolved Hide resolved
docs/Tutorials/CCE.md Outdated Show resolved Hide resolved
docs/Tutorials/CCE.md Outdated Show resolved Hide resolved
@knelli2 knelli2 force-pushed the reduce_cce_combine_h5 branch from 5327598 to 3da4b46 Compare December 13, 2024 20:23
@knelli2
Copy link
Contributor Author

knelli2 commented Dec 13, 2024

@nilsdeppe I pushed a fixup and added a new commit that adds this small exec which writes the collocation points to a text file to the statically deployed execs. Then in the last commit I just additionally renamed the new things I added

Copy link
Member

@nilsdeppe nilsdeppe left a comment

Choose a reason for hiding this comment

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

LGTM! Please go ahead and squash, including the minor clarification request!

@@ -199,6 +202,14 @@ supports \f$\ell\in[4, 32]\f$.
\snippet Test_Spherepack.cpp spectre_cce_grid_point_locations
</details>

Alternatively, if your code can read in grid points from a text file, you can
Copy link
Member

Choose a reason for hiding this comment

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

text file or h5? Because the -o below is with a .h5. You can do this when you squash :)

The worldtube data can now be one of
MetricModal, MetricNodal, BondiModal, BondiNodal. Also, the executable
can now combine multiple H5 files into one.
@knelli2 knelli2 force-pushed the reduce_cce_combine_h5 branch from 3da4b46 to 2fad23b Compare December 13, 2024 22:12
@knelli2 knelli2 removed the dependent Needs a different PR to be merged in first label Dec 13, 2024
@nilsdeppe nilsdeppe enabled auto-merge December 13, 2024 22:24
@nilsdeppe nilsdeppe added the auto-merge GitHub's auto-merge has been enabled for this PR. label Dec 13, 2024
@knelli2 knelli2 mentioned this pull request Dec 13, 2024
13 tasks
@nilsdeppe nilsdeppe merged commit e7e48dc into sxs-collaboration:develop Dec 14, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge GitHub's auto-merge has been enabled for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants