GH+CCE for a gauge wave #4271

Sizheng-Ma · 2022-10-05T01:01:38Z

Proposed changes

This PR adds a GH+CCE executable for a gauge wave (without horizons). It lays the foundation for CCM. The system is not compatible with the local time stepping.

Upgrade instructions

I add one more template to SendGhWorldtubeData so that it can distinguish between LTS and GTS. Please remember to update your CceWorldtubeTarget @knelli2

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 · 2022-10-06T18:55:57Z

src/ParallelAlgorithms/Interpolation/Callbacks/SendGhWorldtubeData.hpp

 /// a TimeStepId.
-template <typename CceEvolutionComponent, bool DuringSelfStart>
+template <typename CceEvolutionComponent, bool DuringSelfStart, bool LTS>


I see why you had to add the extra template, but DuringSelfStart and LTS serve the same purpose here so could you combine them somehow? Maybe name it NeedsInterfaceManager? Then whatever calls this is responsible for passing the correct bool.

I'd think both DuringSelfStart and LTS are wanted because we need to distinguish self-start from evolution even when the interface manager is used (Cce::Actions::ReceiveGhWorldtubeData takes DuringSelfStart other than NeedsInterfaceManager).

Ahh I see, then I think this LTS template is ok.

knelli2 · 2022-10-06T18:57:44Z

src/Evolution/Executables/GeneralizedHarmonic/GeneralizedHarmonicBase.hpp

 template <typename InitialData>
 constexpr auto make_default_phase_order() {


Capitalize reorder in the commit message please.

knelli2 · 2022-10-06T18:59:14Z

src/Evolution/Executables/GeneralizedHarmonic/EvolveGeneralizedHarmonicCce.hpp

+template <typename Metavariables>
+class CProxy_ConstGlobalCache;


What is this? We don't have a ConstGlobalCache

knelli2 · 2022-10-06T18:59:52Z

src/Evolution/Executables/GeneralizedHarmonic/EvolveGeneralizedHarmonicCce.hpp

+
+  template <bool DuringSelfStart, bool LTS>


If you make the above change about the LTS template to SendGhWorldtubeData, I don't think you need this LTS template. Just use the local_time_stepping member that's defined at the top of the metavariables.

For the SendGhWorldtubeData template, I think you can use tmpl::or_v<DuringSelfStart, local_time_stepping>.

I still think you can get rid of this LTS template. Just use the local_time_stepping bool directly inside the target.

knelli2 · 2022-10-06T19:16:19Z

tests/InputFiles/GeneralizedHarmonic/GaugeWave3DCce.yaml

+# Executable: EvolveGhCceGaugeWave
+# Check: parse;execute
+# Timeout: 8


I'd remove the execute part. This takes a while. Parsing should be ok for now

knelli2

Ok all changes look good! Please squash and rebase.

Seems like GCC 9 is still running out of memory on CI...not much I think we can do about that at the moment. One idea could be, for the time being, to just not include this target when building the tests

knelli2 · 2022-10-17T20:23:07Z

@Sizheng-Ma Once #4299 has been merged, you'll have to rebase and fix the #includes.

knelli2 · 2022-11-30T02:12:16Z

@Sizheng-Ma This needs a rebase on develop. Several things have changed over the past month or so.

In order to have CI actually pass and not run out of memory trying to build this executable, please remove the input file you have. If there isn't an input file in the test directory, then it won't build the executable. Please make sure this executable builds on your own branch before pushing. I'll also check that it will build in a couple places before I approve. This will just be a temporary fix until we can reduce the memory usage during compile time.

Sizheng-Ma · 2022-12-08T00:50:31Z

@Sizheng-Ma This needs a rebase on develop. Several things have changed over the past month or so.

In order to have CI actually pass and not run out of memory trying to build this executable, please remove the input file you have. If there isn't an input file in the test directory, then it won't build the executable. Please make sure this executable builds on your own branch before pushing. I'll also check that it will build in a couple places before I approve. This will just be a temporary fix until we can reduce the memory usage during compile time.

I've updated the branch and removed the input file. @knelli2

knelli2

This is still dependent on #4466, but I've built both of these executables locally on this branch and after rebasing on the current develop, so looks good!

nilsdeppe

You can squash immediately. Also please rebase on the latest develop to make sure you have some recent changes to the GH system

nilsdeppe · 2022-12-15T17:33:27Z

src/Evolution/Systems/Cce/Callbacks/SendGhWorldtubeData.hpp

@@ -41,7 +42,7 @@ namespace callbacks {
 /// \note This callback requires the temporal ID in an InterpolationTargetTag be
 /// a TimeStepId.
 template <typename CceEvolutionComponent, typename InterpolationTargetTag,
-          bool DuringSelfStart>
+          bool DuringSelfStart, bool LTS>


LTS -> LocalTimeStepping

nilsdeppe · 2022-12-15T17:33:32Z

tests/Unit/NumericalAlgorithms/Interpolation/Test_SendGhWorldtubeData.cpp

@@ -28,7 +29,7 @@

 namespace {

-template <typename InterpolationTargetTag>
+template <typename InterpolationTargetTag, bool LTS>


LTS -> LocalTimeStepping

nilsdeppe · 2022-12-15T17:33:59Z

tests/Unit/NumericalAlgorithms/Interpolation/Test_SendGhWorldtubeData.cpp

  using with_these_simple_actions = tmpl::list<test_receive_gh_data>;
  using chare_type = ActionTesting::MockArrayChare;
  using array_index = size_t;
  using phase_dependent_action_list = tmpl::list<
      Parallel::PhaseActions<Parallel::Phase::Initialization, tmpl::list<>>>;
 };

+template <bool LTS>


LTS -> LocalTimeStepping

nilsdeppe · 2022-12-15T17:34:07Z

tests/Unit/NumericalAlgorithms/Interpolation/Test_SendGhWorldtubeData.cpp

-    "Unit.NumericalAlgorithms.Interpolation.SendGhWorldtubeDataCallback",
-    "[Unit][Cce]") {
-  MAKE_GENERATOR(gen);
+template <bool LTS, typename Generator>


LTS -> LocalTimeStepping

Done! Cheers.

knelli2 self-requested a review October 5, 2022 01:58

Sizheng-Ma force-pushed the GaugeWave+CCE+GTS branch from 058b9e5 to f48ba9f Compare October 5, 2022 14:49

knelli2 requested changes Oct 6, 2022

View reviewed changes

Sizheng-Ma force-pushed the GaugeWave+CCE+GTS branch from f48ba9f to ac8186e Compare October 7, 2022 03:54

knelli2 reviewed Oct 12, 2022

View reviewed changes

Sizheng-Ma force-pushed the GaugeWave+CCE+GTS branch 2 times, most recently from ff7bd41 to 5f1d85f Compare October 17, 2022 17:00

knelli2 mentioned this pull request Dec 7, 2022

Decouple metavariables and LTS in step chooser simple tags #4466

Merged

3 tasks

Sizheng-Ma added the dependent Needs a different PR to be merged in first label Dec 7, 2022

Sizheng-Ma force-pushed the GaugeWave+CCE+GTS branch 2 times, most recently from 7a81a6f to deecf2b Compare December 8, 2022 00:47

Sizheng-Ma closed this Dec 8, 2022

Sizheng-Ma reopened this Dec 8, 2022

knelli2 previously approved these changes Dec 8, 2022

View reviewed changes

Sizheng-Ma dismissed knelli2’s stale review via 967ef2b December 12, 2022 02:16

Sizheng-Ma force-pushed the GaugeWave+CCE+GTS branch from deecf2b to 967ef2b Compare December 12, 2022 02:16

Sizheng-Ma removed the dependent Needs a different PR to be merged in first label Dec 12, 2022

knelli2 previously approved these changes Dec 12, 2022

View reviewed changes

nilsdeppe requested changes Dec 15, 2022

View reviewed changes

knelli2 and others added 5 commits December 15, 2022 14:24

Send GH data to CCE Action

7a96583

Change callback to avoid interface manager

f3e87a5

Make CharacteristicExtractDefaults compatible with GH+CCE

7b50a8e

Reorder the phase list

63295e5

Add GH+CCE executable for a gauge wave

de3ee71

Sizheng-Ma dismissed knelli2’s stale review via de3ee71 December 15, 2022 23:22

Sizheng-Ma force-pushed the GaugeWave+CCE+GTS branch from 967ef2b to de3ee71 Compare December 15, 2022 23:22

knelli2 approved these changes Dec 15, 2022

View reviewed changes

nilsdeppe approved these changes Dec 16, 2022

View reviewed changes

nilsdeppe merged commit e44a3ce into sxs-collaboration:develop Dec 16, 2022

Sizheng-Ma deleted the GaugeWave+CCE+GTS branch December 16, 2022 23:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH+CCE for a gauge wave #4271

GH+CCE for a gauge wave #4271

Sizheng-Ma commented Oct 5, 2022

knelli2 Oct 6, 2022

Sizheng-Ma Oct 7, 2022

knelli2 Oct 11, 2022

knelli2 Oct 6, 2022

knelli2 Oct 6, 2022

knelli2 Oct 6, 2022

knelli2 Oct 11, 2022

Sizheng-Ma Oct 11, 2022

knelli2 Oct 6, 2022

knelli2 left a comment

knelli2 commented Oct 17, 2022

knelli2 commented Nov 30, 2022

Sizheng-Ma commented Dec 8, 2022 •

edited

Loading

knelli2 left a comment

nilsdeppe left a comment

nilsdeppe Dec 15, 2022

nilsdeppe Dec 15, 2022

nilsdeppe Dec 15, 2022

nilsdeppe Dec 15, 2022

Sizheng-Ma Dec 15, 2022

		template <typename InitialData>
		constexpr auto make_default_phase_order() {

		template <typename Metavariables>
		class CProxy_ConstGlobalCache;

GH+CCE for a gauge wave #4271

GH+CCE for a gauge wave #4271

Conversation

Sizheng-Ma commented Oct 5, 2022

Proposed changes

Upgrade instructions

Code review checklist

Further comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knelli2 left a comment

Choose a reason for hiding this comment

knelli2 commented Oct 17, 2022

knelli2 commented Nov 30, 2022

Sizheng-Ma commented Dec 8, 2022 • edited Loading

knelli2 left a comment

Choose a reason for hiding this comment

nilsdeppe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sizheng-Ma commented Dec 8, 2022 •

edited

Loading