gortiz opened a new issue, #19814: URL: https://github.com/apache/pinot/issues/19814
`generateArray` computes the length of a floating point sequence as `(int) ((end - start) / inc + 1)`. Decimal steps such as `0.1` are not exact in binary, so `(end - start) / inc` often lands just below the integer it should be, and the cast then drops the last element of the range. Adding 1 before the cast hides the error in some cases. For example, `(0.3 - 0.1) / 0.1` is `1.9999999999999998`, and adding 1 rounds it up to exactly `3.0`. It does not help when the error is larger than half an ulp of the sum. Some ranges that return the wrong number of elements today: | Query | Expected | `double` path | `float` path | `(end - start) / inc` in double | |---|---|---|---|---| | `generateArray(0.1, 0.7, 0.1)` | 7 | 6 | 6 | 5.999999999999999 | | `generateArray(0.7, 0.1, -0.1)` | 7 | 6 | 6 | 5.999999999999999 | | `generateArray(0, 0.3, 0.1)` | 4 | 3 | 4 | 2.9999999999999996 | | `generateArray(0, 0.6, 0.1)` | 7 | 6 | 7 | 5.999999999999999 | | `generateArray(0, 0.7, 0.1)` | 8 | 7 | 8 | 6.999999999999999 | | `generateArray(0, 0.9, 0.3)` | 4 | 4 | 3 | 3.0 | Some ranges that work, but only by luck: `0.1..0.3 by 0.1`, `0..1 by 0.1` and `1..2.2 by 0.1`. Integer and long sequences are not affected. I suggest computing the number of steps, and snapping it to the nearest integer when the difference is within the rounding error of the arguments. Otherwise, floor it as today: ```java double steps = (end - start) / inc; double nearestSteps = Math.rint(steps); // For float arguments, use Math.ulp(float) so the tolerance matches the precision of the input type. double roundingError = 2 * (Math.ulp(start) + Math.ulp(end) + Math.abs(nearestSteps) * Math.ulp(inc)) / Math.abs(inc); long fullSteps = (long) (Math.abs(steps - nearestSteps) <= roundingError ? nearestSteps : Math.floor(steps)); int length = (int) fullSteps + 1; ``` The tolerance scales with the magnitude of the arguments, so `1e6 + 0.1 .. 1e6 + 0.3 by 0.1` works too. An end that really falls between two steps is still excluded: `generateArray(0, 0.2999, 0.1)` returns 3 elements. I tried this locally and it returns the expected length for every row in the table above. A fixed relative epsilon (say `1e-9`) is simpler, but it is too tight for `float` arguments, whose relative error is around `1e-7`. Exact decimal arithmetic on the literal text would be ideal, but the multi-stage engine receives the arguments as `double`s, so the text is no longer available there. There is a related question: the elements are computed as `start + i * inc`, so the last element of `generateArray(0.1, 0.3, 0.1)` is `0.30000000000000004`, slightly past `end`. We could clamp the last element to `end`, but that is a separate behavior change. I found this while working on #19552, which makes `generateArray` available in the multi-stage engine and shares a single implementation between both engines. That PR keeps the current length formula, so it does not change any results. @xiangfu0 @Jackie-Jiang I'm pinging you because you requested and reviewed the original function (#11786, #12293). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
