[Einstein Toolkit] #1906: CarpetRegrid2: possible off-by-one error when using regrid_every parameter
#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
#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 Resolution: | Keywords: CarpetRegrid2 ----------------------+-----------------------------------------------------
Comment (by eschnett):
It is possible that there is an off-by-one error, however:
The first iteration, right after setting up initial conditions, is iteration 1. Regridding occurs before the evolution step, and thus the first time it can occur is at iteration 1 (i.e. before the first time step).
Iteration 0 is setting up initial conditions. There can be regridding as well while setting up initial conditions, but this is governed by slightly different rules, so let's disregard this here.
Maybe it is just how iterations are counted that is confusing? With two levels, you have: * it=0: initial conditions * it=1: coarse grid step * it=1: fine grid step * it=2: fine grid step * it=2: restriction to coarse grid * it=3: coarse grid step * it=3: fine grid step * it=4: fine grid step * it=4: restriction to coarse grid
etc.
#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 Resolution: | Keywords: CarpetRegrid2 ----------------------+-----------------------------------------------------
Comment (by bmundim):
Replying to [comment:1 eschnett]:
It is possible that there is an off-by-one error, however:
The first iteration, right after setting up initial conditions, is
iteration 1. Regridding occurs before the evolution step, and thus the first time it can occur is at iteration 1 (i.e. before the first time step).
Then why it advances time/iteration before regridding? Let's say we have just entered the Evolve routine. We can safely assume we are starting at least the first iteration. If I followed your argument right above, then we could step on the block calling CallRegrid and once we are out of it, call the AdvanceTime routine, no?
Iteration 0 is setting up initial conditions. There can be regridding as
well while setting up initial conditions, but this is governed by slightly different rules, so let's disregard this here.
Maybe it is just how iterations are counted that is confusing? With two
levels, you have:
- it=0: initial conditions
- it=1: coarse grid step
- it=1: fine grid step
- it=2: fine grid step
- it=2: restriction to coarse grid
- it=3: coarse grid step
- it=3: fine grid step
- it=4: fine grid step
- it=4: restriction to coarse grid
etc.
Definitely I might have misunderstood the iteration labels. Let me explain how I understand. Correct me if I am wrong. First, the way you labelled the iteration in your example above assumes max_refinement_levels = num_levels_1 = 2. The iteration labels are very different if max_refinement_levels != num_levels_1. Let's say max_refinement_levels = 3. Then the finest possible grid of your simulation would need 2 to (3-1) = 4 iterations to reach one coarse iteration of level 0 grid. Since this number is the maximum one and we need a way to label these iterations, carpet labels the iterations sequentially for this maximum refinement level. The next coarser grid would need 2 iterations and then will exist only on iterations 0, 2, 4, and the coarser level will exist only on iterations 0, 4, assuming we take only one coarse step (one step of level 0 grid). Is this way of reading correct?
Now, let's say we keep the order AdvanceTime and then CallRegrid on Evolve routine. I still find hard to understand why we would enforce regrid based on the previous iteration and not the current one. I would change statements such as
{{{ (cctk_iteration - 1) % regrid_every == 0 }}}
to
{{{ cctk_iteration % regrid_every == 0 }}}
as in line 707 of CarpetRegrid2/src/regrid.cc. Why would that not be correct?
Thanks!
#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 Resolution: | Keywords: CarpetRegrid2 ----------------------+-----------------------------------------------------
Comment (by eschnett):
Yes, your interpretation of my AMR timeline is correct.
Currently, when CallRegrid is called during Evolve, cctk_iteration is always of the form (i %N ==1), where i is the current iteration, and N is the "iteration step size" of the current level. For example, if the coarsest grid is evolved every 4 iterations, then cctk_iteration has the values {1, 5, 9, 13, ...} when CallRegrid is called. Thus the expression (cctk_iteration % regrid_every == 0) would never trigger.
I believe that you would find things easier to understand if iterations were numbered starting at 0. Alas, they are numbered starting at 1. Thus you can define a new counter cctk_iteration0, can always set it via cctk_iteration0 = cctk_iteration - 1, and can then use it: {{{ cctk_iteration0 % regrid_every == 0 }}} This is correct for regridding.
Note that the second half of the coarse grid step occurs after both fine grid steps have completed, and thus occurs at a later iteration (sic!). This was necessary to ensure that cctk_iteration never jumps backwards, which confused some thorns. This, however, does not concern regridding, but it does concern output, as output occurs at the end of a time step. (Regridding occurs in the beginning.)
#1906: CarpetRegrid2: possible off-by-one error when using regrid_every parameter ----------------------+--------------------------------- Reporter: bmundim | Owner: Erik Schnetter Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: CarpetRegrid2 ----------------------+---------------------------------
Comment (by Roland Haas):
Erik, Bruno: can this ticket be closed? It looks like this was mostly a misunderstanding of how iterations are used and when regrid is called.
trac@lists.einsteintoolkit.org
-
Einstein Toolkit