Skip to content

Errors in edge cases of TimeBins

Daniel Lyons requested to merge error-in-edge-cases into main

Allie pointed out two significant issues which could both be detected by looking for situations where the sum of the TimeBins did not match the duration * iteration count (total time) presented to construct it. In general, this property should hold.

The first issue was that in many cases there was a small discrepancy between the total time and the sum of the bins. The second issue was that in cases with wraparound, this error became enormous and unpredictable (large and small).

This MR reworks the algorithm to use a 48-hour window instead of trying to compute wraps intelligently. Basically, we used to do a bunch of mod-24 math and tricks, and now we instead notice if the end is earlier than the start and add 24 to it. Then we do the math the easy way, except at the very end when we are computing the time for (say) 3:00 AM, we also include the time at 24+3=27:00 (3 AM the next day). This seems to produce correct answers, at least insofar as the old tests still pass and the new ones are not broken.

Also updated is the comparison notebook, which now includes a way of detecting when the total times do not match between the algorithms, which currently shows no results (as it should).

Merge request reports