- removed comment
NewRad's extrapolation methods for Gammas expect ghost zones to be valid
Issue #2182
closed
The current stencil size for NewRad's extrapolation of the Xt (contracted Christoffel tensors) variables is 4 and the code checks for a large enough grid like so (extrap.cc line 61ff)
if (dir[d]<0) {
assert(bmax[d] + 4 <=
(cctkGH->cctk_bbox[2*d+1] ? imax[d] : cctkGH->cctk_lsh[d]));
} else if (dir[d]>0) {
assert(bmin[d] - 4 >= (cctkGH->cctk_bbox[2*d] ? imin[d] : 0));
}
which checks (eg for dir[d] > 0 ie an upper boundary) that, unless the lower boundary is a grid boundary, that there are 4 points between bmin and the beginning of the patch (0).
This therefore assumes that ghost points (points 0...cctk_nghostzones) are valid since they will be used if the grid is small enough (less than 4+2*cctk_nghostzones).
Currently this is not ensured by McLachlan and only points in the interior are valid.
This causes the attached parfile to produce poison in the output if run with 2 MPI ranks.
A fix would be to add a SYNC: ML_Gamma to ML_BSSN's ML_BSSN_InitialADMBase2Interior routine or to make the check in NewRad stricter, requiring that there are at least 4 points in the interior (which would also make the check simpler).
My feeling would be that this also explains NaNs that people have seen when running a TOV simulation with many MPI ranks.
I attach a sample parfile as well as sample output.
Keyword: ML_BSSN
Keyword: NewRad
Comments (9)
-
reporter
-
- removed comment
What is the proposed change to the Kranc source file? Please make sure to test this thoroughly, especially with multipatch, e.g. for the BBH gallery example.
-
reporter
- removed comment
I have to see how to actually make this change in the Kranc source. What one needs is a
SYNC: ML_Gammato ML_BSSN's schedule block forML_BSSN_InitialADMBase2Interior. Suggestions on how to achieve this best would be very welcome.In theory as
SYNCshould always be harmless. I will run the BBH gallery example for a bit to make sure though. I have checked (before even creating the ticket) that adding the SYNC lets me run the previously failing parfile (which has no multipatch and no mesh refinement so is far from a production) without failures. -
reporter
- changed status to open
- removed comment
I have created a pull request with the fix, namely changing
InteriorNoSynctoInteriorin McLachlan_BSSN.m which SYNCs ML_Gamma, ML_dtalpha and ML_dtbeta which are exactly the ones thatExtrapolateGammaextrapolates.I have run the GW150914 gallery example for 256 iterations and verified that the checkpoints at it=0 and it=256 agree (bitwise, other than the parameters group) between executables compiled using
InteriorNoSyncandInterior.The pull request can be found here: https://bitbucket.org/einsteintoolkit/mclachlaan/pull-requests/5/rhaas-sync-gamma/diff
-
- removed comment
(The actual pull request is https://bitbucket.org/einsteintoolkit/mclachlan/pull-requests/5/rhaas-sync-gamma/)
Please separate the Kranc source file commit from the code regeneration commit, otherwise merging is a nightmare.
Other than that, it's good. I expect the tests will pass, but if they don't, we can go back and figure out why.
-
- removed comment
Sorry, ignore the above, you already made separate commits. I was confused because bitbucket shows the result of the whole branch by default. PR accepted and merged.
-
reporter
- changed status to resolved
- removed comment
Thank you. No idea where the extra a in mclachlaan came from (must have hit "a" when copy&pasting the URL from the browser).
-
reporter
- removed comment
NaN's in TOV simulations were reported in
#2008. -
reporter
- edited description
- changed status to closed
- Log in to comment
- Assignee
- –
- Type
- Priority
- Status
- closed
- Component
- EinsteinToolkit thorn
- Milestone
- ET_2018_08
- Version
- development version
- Votes
- 0
- Watchers
- 0
In the phone call http://lists.einsteintoolkit.org/pipermail/users/2018-August/006459.html it was decided to add the SYNC statement to
ML_BSSN_InitialADMBase2Interior. This should fix the issue.