[ET Trac] #2653: Carpet PreSync triggering syncs for all refinement levels unnecessarily

[Samuel Cupp]

#2653: Carpet PreSync triggering syncs for all refinement levels unnecessarily

 Reporter: Samuel Cupp
   Status: new
Milestone: 
  Version: 
     Type: bug
 Priority: major
Component: Carpet

In Carpet, the function `PreSyncGroups` calls `SyncProlongateGroups` to automatically sync variables based on the reads and writes. I include the function below.

```
void PreSyncGroups(cFunctionData *attribute, cGH *cctkGH,
                   const std::vector<int> &pre_groups) {
  DECLARE_CCTK_PARAMETERS;

  assert(not CCTK_EQUALS(presync_mode, "off") and
         not CCTK_EQUALS(presync_mode, "warn-only"));

  if (pre_groups.empty())
    return;

  if (reflevel > 0) {
    // recurse to check that all coarsers levels are properly SYNCed
    CCTK_REAL previous_time = cctkGH->cctk_time;
    const int parent_reflevel = reflevel - 1;
    BEGIN_GLOBAL_MODE(cctkGH) {
      ENTER_LEVEL_MODE(cctkGH, parent_reflevel) {
        cctkGH->cctk_time = tt->get_time(mglevel, reflevel, timelevel);
        PreSyncGroups(attribute, cctkGH, pre_groups);
      }
      LEAVE_LEVEL_MODE;
    }
    END_GLOBAL_MODE;
    cctkGH->cctk_time = previous_time;
  }

  // ask Carpet to do the SYNC, this will apply BC as well
  SyncProlongateGroups(cctkGH, pre_groups, attribute);
}
```

Starting at line 11, the code claims to “check that all coarser levels are properly SYNCed”. However, this “check” amounts to blindly syncing every coarser refinement level! This is very inefficient and a huge performance problem that grows with the number of refinement levels. With a parfile for a binary black hole simulation with BaikalVacuum using 12 refinement levels, I found that each iteration was taking 6 times longer. For now in my work, I am commenting lines 11-24 out and assuming that coarser levels are fine. However, this definitely needs to be fixed in a more ‘proper’ way.

This ties into a similar issue I ran into when I didn’t have boundary conditions registered \(due to my own mistake\). Since boundaries weren’t getting filled, every READ\(everywhere\) would trigger sync\+BCs, but there were no BCs to apply. Therefore, even if it _should_ have been synced already, it would sync _again_ with the next READ\(everywhere\) because there were no BCs applied. As an aside, I feel that this case might have needed to generate an error somewhere, as a READ\(everywhere\) was running fine even though it was only valid for interior \+ ghosts.

I propose that a reasonable fix would be to add a check in SyncProlongateGroups to see what is actually needed. This function eventually ends up with a list of `goodgroups` that are to be synced and have boundary conditions applied. Once this preliminary list is constructed, PreSync should further refine this list into those that need syncs and those that need BCs using validities. Then, it can pass two different lists to each, if necessary. I don’t know if there’s another way for PreSync simulations to get to SyncGroups other than through this function, so there may have to be a check in SyncGroups if that is the case.

--
Ticket URL: https://bitbucket.org/einsteintoolkit/tickets/issues/2653/carpet-presync-triggering-syncs-for-all
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.einsteintoolkit.org/pipermail/trac/attachments/20221005/3882d048/attachment.html