CarpetMask should not be scheduled before SphericalSurface_Set

[Gabriele Bozzola]

CarpetMask wants sf_info from SphericalSurface, which is initially not available. Looking at the scheduler, I can see why this is happening (relevant section):

GROUP MaskBase_SetupMask: Set up the weight function
          GROUP MaskBase_SetupMaskAll: Set up the weight function
            CarpetReduce::MaskBase_AllocateMask: [global] Allocate the weight function
            CarpetReduce::MaskBase_InitMask: [global] [loop-local] Initialise the weight function
            GROUP SetupIMaskInternal: Set up the integer weight function (schedule other routines in here)
              CarpetReduce::CoordBase_SetupMask: [global] [loop-local] Set up the outer boundaries of the weight function
              CarpetReduce::CarpetMaskSetup: [global] [loop-singlemap] Set up the weight function for the restriction regions
            GROUP SetupIMask: Set up the integer weight function (schedule other routines in here)
              CarpetMask::CarpetExcludedSetup: [global] [loop-local] Set up the weight function for the excluded regions
              CarpetMask::CarpetSurfaceSetup: [global] [loop-local] Set up the weight function for the excluded spherical surfaces
            CarpetReduce::MaskBase_SetMask: [global] [loop-local] Set the weight function
            GROUP SetupMask: Set up the real weight function (schedule other routines in here)
            CarpetReduce::MaskBase_TestMask: [global] Test the weight function
        SphericalSurface::SphericalSurface_Set: [global] Set surface radii to be used for initial setup in other thorns
        GROUP SphericalSurface_HasBeenSet: Set the spherical surfaces before this group, and use it afterwards
          SphericalSurface::SphericalSurface_CheckState: [global] Test the state of the spherical surfaces

CarpetMask is scheduled before SphericalSurface_Set, which is what sets sf_info. This is in the BASEGRID bin. As a result, masks are possibly not correctly set at iteration 0, and the run fails when presync is set to mixed-error.

[Roland Haas]

Can you create a pull request (for CarpetMask) to fix the schedule?

[Gabriele Bozzola]

The function in CarpetMask are scheduled in SetupIMask, which is a group provided by CarpetReduce.

If I look at the schedule.ccl of CarpetReduce, I see

SCHEDULE GROUP MaskBase_SetupMask AT basegrid AFTER (SpatialCoordinates SphericalSurface_Setup)

Is the correct fix to add Spherical_Surface_Set (or SphericalSurface_HasBeenSet) among the AFTER here?

[Roland Haas]

yup. Might even be that one should change SphericalSurface_Setupto SphericalSurface_Set in case SphericalSurface_Setup does not actually initialize sf_info (and thus scheduling relative it it is pointless). That would be future enhancement though (and needs some more checking for possible unanticipated side effects first).

[Gabriele Bozzola]

https://bitbucket.org/eschnett/carpet/pull-requests/52/reduce-schedule-maskbase_setupmask-after

[Gabriele Bozzola]

I added SphericalSurface_HasBeenSet, however I think that the correct fix should be to substitute `SphericalSurface_HasBeenSet` to SphericalSurface_Setup. My impression is that the group SphericalSurface_HasBeenSet is there exactly as a placeholder to ensure that spherical surfaces exist.

[Gabriele Bozzola]

I found this out possible problem. SphericalSurface_Set does not specify an AFTER statement for SphericalSurface_Setup. When I was messing around with the scheduler trying to fix this bug, I had cases in which Set was scheduled before Setup, leading to crashes.

‌

[Gabriele Bozzola]

PR: https://bitbucket.org/cactuscode/cactusnumerical/pull-requests/15/sphericalsurface-ensure-that-set-is-after