[ET Trac] [Einstein Toolkit] #1906: CarpetRegrid2: possible off-by-one error when using regrid_every parameter

[Einstein Toolkit]

#1906: CarpetRegrid2: possible off-by-one error when using regrid_every parameter
---------------------------+------------------------------------------------
 Reporter:  bmundim        |       Owner:  eschnett           
     Type:  defect         |      Status:  new                
 Priority:  major          |   Milestone:                     
Component:  Carpet         |     Version:  development version
 Keywords:  CarpetRegrid2  |  
---------------------------+------------------------------------------------
 I was working on a par file for a quick regridding test and found out an
 unexpected behaviour regarding regridding. Basically I want a par file
 with a few iterations such that it adds a refinement level every 4
 iterations for example. I set the following parameters then:

 {{{
 CarpetRegrid2::add_levels_automatically      = "yes"
 CarpetRegrid2::regrid_every  = 4
 }}}

 for a grid structure as such:

 {{{
 Cactus::cctk_itlast = 10
 CoordBase::xmin = -0.5
 CoordBase::ymin = -0.5
 CoordBase::zmin = -0.5
 CoordBase::xmax =  0.5
 CoordBase::ymax =  0.5
 CoordBase::zmax =  0.5
 CoordBase::ncells_x   =  16
 CoordBase::ncells_y   =  16
 CoordBase::ncells_z   =  16
 Carpet::max_refinement_levels    = 3
 CarpetRegrid2::num_centres      = 1
 CarpetRegrid2::active_1      = "yes"
 CarpetRegrid2::num_levels_1  = 1
 CarpetRegrid2::radius_1[1]   = 0.12
 CarpetRegrid2::radius_1[2]   = 0.04
 }}}

 I was expecting then regridding to happen at iterations 4 and 8, however
 if you run the attached par file, a modification of balsara shocktube
 test, you see that the level additions actually happen at iterations 5 and
 9 instead:

 {{{
 ...
 INFO (CarpetRegrid2): Increasing number of levels of centre 1 to 2 (it=5)
 ...
 INFO (CarpetRegrid2): Increasing number of levels of centre 1 to 3 (it=9)
 }}}

 which is apparently an off-by-one kind of error. I tracked down the code
 producing these messages and it comes from function
 CarpetRegrid2_RegridMaps at CarpetRegrid2/src/regrid.cc, lines 745 to 747.
 In order for that piece of code to execute we do have to have do_recompose
 set to true. Strangely the condition to set it, compares the previous
 iteration to the regrid_every parameter on line 707 of the same file:

 {{{
 (cctk_iteration - 1) % regrid_every == 0
 }}}

 Investigating if this actually makes sense I was led to what I think it
 might be the source of the problem on line 58 of Carpet/src/Evolve.cc.
 Time and iteration is advanced before calling CallRegrid. Skimming over
 the AdvanceTime routine it is not clear to me that it really needs to be
 called before CallRegrid. If it is really needed then why the condition to
 regrid or not is not taken on the current updated iteration as in:

 {{{
 cctk_iteration % regrid_every == 0
 }}}

 Thanks,
 Bruno

-- 
Ticket URL: <https://trac.einsteintoolkit.org/ticket/1906>
Einstein Toolkit <http://einsteintoolkit.org>
The Einstein Toolkit