#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