Conversation
|
let me redo this... |
|
better |
CarpetX/src/interpolate.cxx
Outdated
| // const int component = mfp.index(); | ||
|
|
||
| min_level_iteration_used = | ||
| std::min(min_level_iteration_used, leveldata.iteration); |
There was a problem hiding this comment.
This test seems to fail because min_level_iteration_used.numerator is long int max, and the test x.num * y.den overflows when den > 1
CarpetX/Arith/src/rational.hxx
Line 196 in e518a3c
There was a problem hiding this comment.
worked around by using -1 to indicate the "unitilalized" case plus a couple of extra workarounds for MPI's MPI_MAX reduction.
A "cleaner" solution would be to make the < comparing not overflow by doing repeating divisions with remainders until a decision has been reached (is slow) or using double-length numbers as intermediaries (that would be fast I think). Both are overkill for this single application though.
There was a problem hiding this comment.
I ran a test that confirmed that the code correctly errors out when checkpointing on unaligned time levels and that interpolation errors out if time interpolation is required. Checkpointing on aligned timelevels works and interpolations not requiring time interpolation also work
There was a problem hiding this comment.
Great. Thanks for checking. That leaves as the only place that is a bit iffy the cast to double in the interpolator check, which reduces things to 53 bits of information (size of the mantissa of a double). However the iterations also need to fit into Cactus' cctk_iteration which is a int so 32bit only anyway and since refinement in time is restricted to powers of two we can exactly represent the rat64 as a double for all cases that Cactus can actually encounter.
|
Addressed reviewer comments. |
2 participants
@eschnett @lwJi @yosef-zlochower here's some sanity checks that will prevent checkpointing and interpolation when not possible.