CarpetX: Change the implementation to Driver. #305

rhaas80 · 2024-08-30T21:17:27Z

~~This involves adding a parameter "periodic" whose only allowed value is "no."~~
This involves adding a parameter "periodic" that must be set to "yes" for any periodicity to be active.
It also involves modifying parameter files, file names, and source files ~~where the name "CarpetX" needs to be replaced by "Driver."~~ to account for the fact that the regrid_error grid function moved from CarpetX to CarpetRegridX.

Parameter files may, but not have to use Driver::periodic instead of CarpetX::periodic since the parameter file parser accepts both implementation and thorn names for restricted parameters.

ActiveThorns = PUGH

PUGH::periodic = yes
Driver::periodic_x = no

This picks up the work in #299

rhaas80 · 2024-08-30T21:22:35Z

Ticket is here https://bitbucket.org/einsteintoolkit/tickets/issues/2813/allow-carpetx-to-implement-driver

TestSymmetries/par/test_symmetry_xperiodic_yperiodic_zperiodic.par

WaveToyX/par/standing.par

WaveToyX/test/presync.par

CarpetX/param.ccl

CarpetXRegrid/README

SpacetimeWaveToyX/par/standing.par

SpacetimeWaveToyX/test/standing/standing.par

StaggeredWaveToyX/par/standing.par

StaggeredWaveToyX/test/standing/standing.par

TestODESolvers/test/test-rkf78.par

rhaas80 · 2024-09-02T14:11:52Z

Will need to add CarpetXRegrid to the thornlist used by the CI testing.

eschnett · 2024-09-10T19:14:40Z

"This involves adding a parameter "periodic" whose only allowed value is "no."". is that still true?

rhaas80 · 2024-09-10T19:21:38Z

"This involves adding a parameter "periodic" whose only allowed value is "no."". is that still true?

not true anymore. It does introduce a parameter periodic but it must be set to "yes" for any periodicity to be applied.

eschnett

There should be checkpoint/recovery tests as well. I think the respective checkpoint files need to be regenerated.

eschnett · 2024-09-10T19:17:03Z

BoxInBox/test/levels1.par

@@ -3,6 +3,7 @@ ActiveThorns = "
    CarpetX
    CoordinatesX
    IOUtil
+    CarpetXRegrid


Can you sort thorn names alphabetically?

eschnett · 2024-09-10T19:18:09Z

CarpetX/configuration.ccl

@@ -2,7 +2,7 @@

 REQUIRES AMReX IOUtil MPI yaml_cpp zlib

-REQUIRES Arith Loop
+REQUIRES Arith Loop CarpetXRegrid


Can you sort requirements alphabetically?

eschnett · 2024-09-10T19:19:08Z

CarpetX/interface.ccl

-IMPLEMENTS: CarpetX
-
-INHERITS: IO
+IMPLEMENTS: Driver


That's it! That's the core of the change! Yay!

eschnett · 2024-09-10T19:21:48Z

CarpetX/param.ccl


-BOOLEAN periodic_y "Periodic"
+BOOLEAN periodic_x "Periodic boundary conditions in x-direction"


Can you add "only takes effect if also periodic = yes or similar?

eschnett · 2024-09-10T19:22:46Z

CarpetX/param.ccl

-
-PRIVATE:
-
+#TODO: reliminate in favor of regrid_error being a ture/false field


Two typos: reliminate, ture.

eschnett · 2024-09-10T19:23:55Z

CarpetXRegrid/schedule.ccl

@@ -0,0 +1,6 @@
+# Schedule definitions for thorn CarpetXRegrid
+SCHEDULE CarpetX_InitError AT basegrid


The function name prefix should be CarpetXRegrid.

eschnett · 2024-09-10T19:26:13Z

SpacetimeWaveToyX/test/standing/standing.par

-CarpetX::periodic_x = yes
-CarpetX::periodic_y = yes
-CarpetX::periodic_z = yes
+Driver::periodic_x = yes


Is a Driver::periodic = yes missing here?

That's in the output directory. The parfile should be deleted there.

eschnett · 2024-09-10T19:26:41Z

StaggeredWaveToyX/test/standing/standing.par

-CarpetX::periodic_x = yes
-CarpetX::periodic_y = yes
-CarpetX::periodic_z = yes
+Driver::periodic_x = yes


Is a Driver::periodic = yes missing here?

That's in the output directory. The parfile should be deleted there.

rhaas80 · 2024-09-10T19:33:16Z

"This involves adding a parameter "periodic" whose only allowed value is "no."". is that still true?

not true anymore. It does introduce a parameter periodic but it must be set to "yes" for any periodicity to be applied.

Turns out that (see updated description) the "adapt to CarpetX implementation change" commits aren't actually required since the parameter file parser will accept both implementation and thorn names for restricted parameters (and private ones as well, which is a bug...).

rhaas80 · 2024-09-10T19:53:37Z

There should be checkpoint/recovery tests as well. I think the respective checkpoint files need to be regenerated.

The TestOuptut/recover-openpmd test passes:

  [10] recover-openpmd
       CarpetX/TestOutput/test/recover-openpmd.par
  Choose test: [0] 
   --> 10

------------------------------------------------------------------------

  Test TestOutput: recover-openpmd
    "CarpetX/TestOutput/test/recover-openpmd.par"

  Issuing mpirun -np 2 /data/rhaas/postdoc/gr/cactus/ET_trunk/exe/cactus_sim /data/rhaas/postdoc/gr/cactus/ET_trunk/arrangements/CarpetX/TestOutput/test/recover-openpmd.par

  Success: 40 files identical

Looking at file md.0 it only has settings for parameters:

CarpetX::periodic_x = no
CarpetX::periodic_y = no
CarpetX::periodic_z = no

which are apparently acceptable (not sure why, http://einsteintoolkit.org/referencemanual/ReferenceManual.html#x1-156000A2 clearly says it needs to be the implementation name "Driver").

The missing Driver::periodic is not caught because it is a new parameter.

This involves adding a parameter "periodic" whose default value is "no." It also involves modifying parameter files, file names, and source files where the name "CarpetX" needs to be replaced by "Driver."

required due to CarpetX implemting "Driver"

…tput

required due to CarpetX implemting "Driver"

rhaas80 force-pushed the carpetx-driver branch from 934cdb7 to cecee6f Compare August 31, 2024 02:19

eschnett requested changes Aug 31, 2024

View reviewed changes

rhaas80 force-pushed the carpetx-driver branch 3 times, most recently from ff5e7da to 46874bf Compare September 3, 2024 20:36

rhaas80 requested a review from eschnett September 5, 2024 21:53

eschnett requested changes Sep 10, 2024

View reviewed changes

stevenrbrandt and others added 2 commits September 11, 2024 16:09

CarpetXRegrid: Move regrid_error to a new thorn.

c8d5631

CI: Add CarpetXRegrid to thorn list

a5db85e

rhaas80 force-pushed the carpetx-driver branch from ca86318 to 0dbe79a Compare September 12, 2024 17:43

rhaas80 assigned stevenrbrandt Sep 12, 2024

rhaas80 requested a review from eschnett September 12, 2024 18:30

stevenrbrandt and others added 12 commits September 12, 2024 13:55

CarpetX: Change the implementation to Driver

4aafc0a

This involves adding a parameter "periodic" whose default value is "no." It also involves modifying parameter files, file names, and source files where the name "CarpetX" needs to be replaced by "Driver."

Algo: remove parameter file and changing data from test output

28234b6

BenchX: sort thorns alphabetically

e2c6348

BenchX: adapt to CarpetX implementation change

e2de22f

required due to CarpetX implemting "Driver"

BoxInBox: adapt to CarpetX implementation change

1a6d14e

required due to CarpetX implemting "Driver"

ErrorEstimator: adapt to CarpetX implementation change

e38b1f2

required due to CarpetX implemting "Driver"

FluxWaveToyX: adapt to CarpetX implementation change

9d9f9ee

required due to CarpetX implemting "Driver"

MovingBoxToy: adapt to CarpetX implementation change

3a31556

required due to CarpetX implemting "Driver"

PoissonX: adapt to CarpetX implementation change

8a6dad2

required due to CarpetX implemting "Driver"

SiMDWaveToyX: adapt to CarpetX implementation change

835f2e1

required due to CarpetX implemting "Driver"

SpacetimeWaveToyX: adapt to CarpetX implementation change

dd10081

required due to CarpetX implemting "Driver"

StaggeredWaveToyX: adapt to CarpetX implementation change

73a1395

required due to CarpetX implemting "Driver"

rhaas80 added 13 commits September 12, 2024 13:55

TestArrayGroup: remove parameter file and changing data from test output

d720bd4

TestBoundaries: adapt to moving regrid error into its own thorn

fc685d1

TestBoxInBox: adapt to CarpetX implementation change

40f9794

required due to CarpetX implemting "Driver"

TestDerivs: adapt to CarpetX implementation change

26d7846

required due to CarpetX implemting "Driver"

TestInterpolate: adapt to CarpetX implementation change

e58adf0

required due to CarpetX implemting "Driver"

TestLoopX: adapt to CarpetX implementation change

2936f89

required due to CarpetX implemting "Driver"

TestNorms: adapt to CarpetX implementation change

81ae7ca

required due to CarpetX implemting "Driver"

TestODESolvers: adapt to CarpetX implementation change

345e527

required due to CarpetX implemting "Driver"

TestODESolvers2: remove parameter file and changing data from test ou…

c83d7ba

…tput

TestOutput: adapt to moving regrid error into its own thorn

6301ec0

TestProlongate: adapt to CarpetX implementation change

007ddb7

required due to CarpetX implemting "Driver"

TestSymmetries: adapt to CarpetX implementation change

7301e44

required due to CarpetX implemting "Driver"

WaveToyX: adapt to CarpetX implementation change

7046df3

required due to CarpetX implemting "Driver"

rhaas80 force-pushed the carpetx-driver branch from 0dbe79a to 7046df3 Compare September 12, 2024 18:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CarpetX: Change the implementation to Driver. #305

CarpetX: Change the implementation to Driver. #305

rhaas80 commented Aug 30, 2024 •

edited

Loading

rhaas80 commented Aug 30, 2024

rhaas80 commented Sep 2, 2024

eschnett commented Sep 10, 2024

rhaas80 commented Sep 10, 2024

eschnett left a comment

eschnett Sep 10, 2024

rhaas80 Sep 10, 2024

eschnett Sep 10, 2024

rhaas80 Sep 10, 2024

eschnett Sep 10, 2024

eschnett Sep 10, 2024

eschnett Sep 10, 2024

eschnett Sep 10, 2024

eschnett Sep 10, 2024

rhaas80 Sep 10, 2024

eschnett Sep 10, 2024

rhaas80 Sep 10, 2024

rhaas80 commented Sep 10, 2024

rhaas80 commented Sep 10, 2024


		BOOLEAN periodic_y "Periodic"
		BOOLEAN periodic_x "Periodic boundary conditions in x-direction"


		PRIVATE:

		#TODO: reliminate in favor of regrid_error being a ture/false field

		@@ -0,0 +1,6 @@
		# Schedule definitions for thorn CarpetXRegrid
		SCHEDULE CarpetX_InitError AT basegrid

CarpetX: Change the implementation to Driver. #305

Are you sure you want to change the base?

CarpetX: Change the implementation to Driver. #305

Conversation

rhaas80 commented Aug 30, 2024 • edited Loading

rhaas80 commented Aug 30, 2024

rhaas80 commented Sep 2, 2024

eschnett commented Sep 10, 2024

rhaas80 commented Sep 10, 2024

eschnett 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

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

rhaas80 commented Sep 10, 2024

rhaas80 commented Sep 10, 2024

rhaas80 commented Aug 30, 2024 •

edited

Loading