Fix rounding error bug in calculation of number of top-level cells

Because a value of 0.99 was used to force rounding down (see changelog), trying to use a top-level cells count of ~100 could lead to rounding to +/-1 from expected value. This more adaptive approach should avoid this issue for arbitrarily large numbers of top-level cells (at least until Ncells^2 approaches the floating point limit).

There is a potential issue if trying to use one top-level cell. I think it might make sense to enforce a minimum value for the tol that I've introduced (could be the previously-used 0.99, for instance, just needs to be less than 1). Thought I'd ask here before adding more logic, though.

changed the description

Thanks. Could you add some parenthesis in the first line? a/b/c is always ambiguous. a/(b/c) or (a/b)/c?

We should try that on some of the hydro tests that use many many more cells and/or non-cube geometries.

added 1 commit

• 2807ad42 - Add parentheses for clarity.

Compare with previous version

@matthieu parenthesized. Is there a test suite I can run (or rather are there instructions, because ./test-driver doesn't seem to get me anywhere useful)? The 1 or small number of top-level cell case is also worth testing.

added bug label

No, no real suite testing everything. That would take weeks. I need to think a bit about what tests matter here. Also whether this can trigger issues in runs where a regrid actually took place and we restart after that. Mostly pure-hydro cases but I don't want to break that.

I don't think that this will affect runs where a re-grid occurs, if I understand the code. The calculation simplifies to (pseudo-code-ish):

if h_max * kernel_gamma * space_stretch > s->cell_min:
  cdim = floor(s->dim / (h_max * kernel_gamma * space_stretch))
else:
  cdim = floor(max_top_level_cells / tol)

I think that a re-grid would only be triggered by h_max becoming large enough? In that case it always goes to the first branch of the if statement and never touches tol.

Thinking about this further, I should put a lower bound on tol, because for small number of cells you'd get things like cdim = floor(max_top_level_cells/0.5) , which is just glitchy rounding in the other direction!

added 1 commit

• 8f1a38c4 - Enforce a lower bound on tolerance.

Compare with previous version

Agreed. I worry though about a restart re-reading values from the param file after a regrid happened in the first part of the runs.