From 292be817b73c5c40d3e7cf43a9318e7bc0993c5e Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 6 Nov 2022 23:56:56 +0100 Subject: [PATCH] Timeline: investigate problem with numeric overflow in fractional arithmetic Extensive tests with corner cases soon highlighted this problem inherent to integer calculations with fractional numbers: it is possible to derail the calculation by numeric overflow with values not excessively large, but using large numbers as denominator. This problem is typically triggered by addition and subtraction, where you'd naively not expect any problems. Thus changed the approach in the normalisation function, relying on an explicitly coded test rather, and performing the adjustment only after conversion back to simple integral micro-tick scale. --- src/lib/rational.hpp | 20 +- src/stage/model/zoom-window.hpp | 47 ++- tests/stage/model/zoom-window-test.cpp | 3 +- wiki/thinkPad.ichthyo.mm | 387 +++++++++++++++++++++---- 4 files changed, 395 insertions(+), 62 deletions(-) diff --git a/src/lib/rational.hpp b/src/lib/rational.hpp index 6a7eef09b..9fb301024 100644 --- a/src/lib/rational.hpp +++ b/src/lib/rational.hpp @@ -42,6 +42,22 @@ ** conversion using `rational_cast (fraction)` is necessary, which ** performs the division of `numerator/denominator` in the target value domain. ** + ** # Perils of fractional arithmetics + ** + ** While the always precise results of integral numbers might seem compelling, the + ** danger of _numeric overflow_ is significantly increased by fractional computations. + ** Most notably, this danger is *not limited to large numbers*. Adding two fractional + ** number requires multiplications with both denominators, which can overflow easily. + ** Thus, for every given fractional number, there is a class of »dangerous counterparts« + ** which can not be added without derailing the computation, leading to arbitrary wrong + ** results without detectable failure. And these problematic counterparts are distributed + ** _over the whole valid numeric range._ To give an extreme example, any numbers of the + ** form `n / INT_MAX` can not be added or subtracted with any other rational number > 1, + ** while being themselves perfectly valid and representable. + ** \par rule of thumb + ** Use fractional arithmetics only where it is possible to control the denominators involved. + ** Never use them for computations drawing from arbitrary (external) input. + ** ** @see zoom-window.hpp ** @see timevalue.hpp */ @@ -64,11 +80,11 @@ namespace util { } // namespace util -/** +/** * user defined literal for constant rational numbers. * \code * Rat twoThirds = 2_r/3; - * \endcode + * \endcode */ inline util::Rat operator""_r (unsigned long long num) diff --git a/src/stage/model/zoom-window.hpp b/src/stage/model/zoom-window.hpp index b0709a50e..af38a9f53 100644 --- a/src/stage/model/zoom-window.hpp +++ b/src/stage/model/zoom-window.hpp @@ -124,6 +124,33 @@ namespace model { { return FSecs{_raw(timeVal), TimeValue::SCALE}; } + + /** + * @return `true` if the given duration can be represented cleanly as µ-ticks. + * @todo should likewise be member of a FSecs wrapper type... + */ + inline bool + isMicroGridAligned (FSecs duration) + { + return 0 == (duration.numerator() * Time::SCALE) + % duration.denominator(); + } + + /////////////////////////////////////////////////////////////////////////////////////////////////////////TICKET #1261 : why the hell did I define time entities to be immutable. Looks like a "functional programming" fad in hindsight + /** @todo we need these only because the blurry distinction between + * lib::time::TimeValue and lib::time::Time, which in turn is caused + * by a lack of clarity in the fundamental conception (see #1261) + */ + inline TimeVar + operator+ (Time const& tval, TimeVar const& tvar) + { + return TimeVar(tval) += tvar; + } + inline TimeVar + operator- (Time const& tval, TimeVar const& tvar) + { + return TimeVar(tval) -= tvar; + } } @@ -136,7 +163,6 @@ namespace model { const uint MAX_PX_WIDTH{1000000}; const FSecs MAX_TIMESPAN{_FSecs(Time::MAX-Time::MIN)}; const FSecs MICRO_TICK{1_r/Time::SCALE}; - const Rat FRACT_ULP{1_r/std::numeric_limits::max()}; } @@ -365,14 +391,17 @@ namespace model { FSecs dur{afterWin_-startWin_}; uint pxWidth = rational_cast (px_per_sec_*dur); dur = Rat(pxWidth) / changedMetric; - dur += MICRO_TICK - FRACT_ULP; // prefer bias towards increased window instead of increased metric dur = min (dur, MAX_TIMESPAN); dur = max (dur, MICRO_TICK); // prevent window going void - if (startWin_<= Time::MAX - Time{dur}) - afterWin_ = startWin_ + Time{dur}; + TimeVar timeDur{dur}; + // prefer bias towards increased window instead of increased metric + if (not isMicroGridAligned (dur)) + timeDur = timeDur + TimeValue(1); + if (startWin_<= Time::MAX - timeDur) + afterWin_ = startWin_ + timeDur; else { - startWin_ = Time::MAX - Time{dur}; + startWin_ = Time::MAX - timeDur; afterWin_ = Time::MAX; } // re-check metric to maintain precise pxWidth @@ -559,10 +588,10 @@ namespace model { return 1_r/2; // then anchor zooming in the middle // use a 3rd degree parabola to favour positions in the middle - FSecs posFactor = FSecs{startWin_-startAll_} / possibleRange; - posFactor = (2*posFactor - 1); // -1 ... +1 - posFactor = posFactor*posFactor*posFactor; // -1 ... +1 but accelerating towards boundraries - posFactor = (posFactor + 1) / 2; // 0 ... 1 + Rat posFactor = FSecs{startWin_-startAll_} / possibleRange; + posFactor = (2*posFactor - 1); // -1 ... +1 + posFactor = posFactor*posFactor*posFactor; // -1 ... +1 but accelerating towards boundaries + posFactor = (posFactor + 1) / 2; // 0 ... 1 return posFactor; } diff --git a/tests/stage/model/zoom-window-test.cpp b/tests/stage/model/zoom-window-test.cpp index 6838ed401..a8faa918c 100644 --- a/tests/stage/model/zoom-window-test.cpp +++ b/tests/stage/model/zoom-window-test.cpp @@ -28,6 +28,7 @@ #include "lib/test/run.hpp" #include "lib/test/test-helper.hpp" #include "stage/model/zoom-window.hpp" +#include "lib/format-cout.hpp"//////////////TODO @@ -68,7 +69,7 @@ namespace test { CHECK (FSecs(10)/3 == 10_r/3); // _r is a user defined literal to denote 64-bit fractional CHECK (Rat(10,3) == 10_r/3); CHECK (Rat(10,3) == boost::rational(10,3)); // using Rat = boost::rational - CHECK (rational_cast (10_r/3) == 3.33333f); // rational_cast calculates division after type conversion + CHECK (rational_cast (10_r/3) == 3.3333333f); // rational_cast calculates division after type conversion verify_simpleUsage(); verify_setup(); diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 56f681c9e..695ee5845 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -38861,7 +38861,7 @@ - + @@ -38926,51 +38926,46 @@ -<html> - <head> + + - - </head> - <body> - <p> - <u>Konkretes Rechenbeispiel:</u> - </p> - <p> - pxWidth = 99999&#160;&#160;(ein Pixel weniger als maximal m&#246;glich) - </p> - <p> + + +

+ Konkretes Rechenbeispiel: +

+

+ pxWidth = 99999  (ein Pixel weniger als maximal möglich) +

+

changedMetric = MAX_ZOOM = 2000000 - </p> - <p> - &#10233; conformWindowToMetric() errechnet - </p> - <p> - dur = 49999+1/2 &#181;Tick - </p> - <p style="margin-top: 0px; margin-bottom: 0px; margin-left: 0px; margin-right: 0px; text-indent: 0px"> - &#55358;&#56370; das wird abgerundet auf 49999 &#181;Tick - </p> - <p> - &#10233; conformMetricToWindow() errechnet - </p> - <p> +

+

+ ⟹ conformWindowToMetric() errechnet +

+

+ dur = 49999+1/2 µTick +

+

+ �� das wird abgerundet auf 49999 µTick +

+

+ +

+

+ ⟹ conformMetricToWindow() errechnet +

+

adjMetric = 99999 / (49999/1000000) = 2000020,0004000080001600032 - </p> - <p style="margin-top: 0px; margin-bottom: 0px; margin-left: 0px; margin-right: 0px; text-indent: 0px"> - &#10233; das ist <b>definitiv gr&#246;&#223;er</b>&#160;als MAX_ZOOM - </p> - <p> +

+

- </p> - <p> - - </p> - <p> - - </p> - </body> -</html> - +

+

+ ⟹ das ist definitiv größer als MAX_ZOOM +

+ + @@ -38992,7 +38987,8 @@ - + + @@ -39014,11 +39010,16 @@ - + - - - + + + + + + + + @@ -39029,8 +39030,288 @@

- - + + + + + + + + + +

+ es kommt eine negative Zahl heraus: IntMax * Scale ⟼ -Scale  (im Nenner) +

+ + + + + + + + + + + + + + + + +

+ läßt sich ein »unbeherrschbarer Summand« konstruieren +

+ + + + + + + + + + + + + + + + +

+ da wird die Rechnung schwierig, denn ich kann es nicht ohne Weiteres mathematisch greifen.... +

+ + + + + + + + + + +

+ Ansatz mit Unbekannten +

+

+ 1/S - 1/M = x / I  < 1/S +

+ + + + + + + + +

+ Ansatz mit Unbekannten führt auf eine nicht-lineare Gleichung, welche + die beiden Unbekannten verknüpft.... +

+

+ mit ULP = 1/M  und I (IntMax) und S (µTick Scale) setze ich an: +

+

+ +

+

+ �� 1/s - 1/M = x / I +

+

+ +

+

+ denn einen Ausdruck x/I kann man grade noch repräsentieren in + fraktionaler Arithmetik. +

+

+ Das fürhrt dann leider zu einger Gleichung, die nicht besonders sexy daherkommt +

+

+
+ �� I·S = I·M - x·M·S +

+

+ +

+

+ also +

+
    +
  • +  x = (I·M - I·S ) / (M·S) = (M - S) / (M·S/I) = I · (M - S)/(M·S) +
    • +
  • +  M = I·S / (I - x·S) = S / (1 - x·S/I) +
    • +
+

+ +

+

+ +

+ + + + + + + + + + + + +

+ denn für x ≔ I/S  ergibt sich x/I = 1/s +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ empirsche Kontrolle +

+

+     const Rat UU = Rat(X/I); +

+

+     SHOW_EXPR (UU); +

+

+     SHOW_EXPR ((UU < MICRO_TICK)); +

+

+     SHOW_EXPR ((Rat(X+1)/I < MICRO_TICK)); +

+

+ +

+

+ Ausgabe... +

+

+ Probe rational_cast<int64_t>(LIM) ? = 9223372036854 +

+

+ Probe UU ? = 9223372036854/9223372036854775807 +

+

+ Probe (UU < MICRO_TICK) ? = true +

+

+ Probe (Rat(X+1)/I < MICRO_TICK) ? = false +

+

+ +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
    +
  • + Wenn der Nenner selbst S ist +
    • +
  • + Wenn der Nenner ein Teiler von S ist +
    • +
  • + wenn der Nenner ein Teiler von S · pxWidth ist +
    • +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -39046,6 +39327,9 @@ + + + @@ -55947,7 +56231,7 @@ - + @@ -56031,6 +56315,9 @@ + + +