MaxFreedomPollard opened a new pull request, #762:
URL: https://github.com/apache/datasketches-java/pull/762
`Util.numDigits(0)` and `Util.numDigits(1)` both return 0, and
`Util.numDigits(1000000000000000000L)` returns 18 for a number with 19 digits.
The method is at
`src/main/java/org/apache/datasketches/common/Util.java:805`:
```java
public static int numDigits(long n) {
if ((n % 10) == 0) { n++; }
return (int) ceil(log(n) / log(10));
}
```
The `n++` is there for exact multiples of ten, where `ceil` otherwise lands
one short. It does not help 0 or 1: both reach `ceil(log(1) / log(10))`, which
is `ceil(0.0)`, which is 0. It also stops working once a long has no exact
double. At n = 1E18 the nudged value 1000000000000000001 rounds back to 1E18,
the ratio comes out at 17.999999999999996, and `ceil` gives 18.
Measured on current main against `Long.toString(n).length()`:
```
n=0 current=0 expected=1
n=1 current=0 expected=1
n=1000000000000000000 current=18 expected=19
```
Everything from 2 to 999999999999999999 is already right, and so is
`Long.MAX_VALUE`.
The fix drops the floating point for an integer loop, exact across the whole
long range. Zero and negatives now return 1, which is what
`LongsAsOrderableStrings.digits` in the test tree already does for `maxValue <=
0`. That method is the same computation and its javadoc caps it below 1E15; the
loop has no ceiling.
The two halves are used together: `numDigits` sizes the pad that
`longToFixedLengthString` and `LongsAsOrderableStrings.getString` apply so
longs rendered as strings sort in numeric order. An under-reported width leaves
the wider values unpadded and breaks that ordering.
Verification on macOS 15 aarch64, Temurin 25.0.4.1, Maven 3.9.16, toolchain
per the README.
With only the new test applied to main, `mvn --toolchains ... test
-Dtest=UtilTest -DfailIfNoTests=false` gives `Tests run: 44, Failures: 1` at
`UtilTest.checkNumDigits:248 expected [1] but found [0]`. The same command with
the fix gives `Tests run: 44, Failures: 0, Errors: 0, Skipped: 0`.
Every in-repo caller of `Util.numDigits` is a test class. Running all of
them,
`-Dtest=UtilTest,KllItemsSketchTest,KllMiscItemsTest,KllHelperTest,KllDirectCompactItemsSketchTest,PartitionBoundariesTest,KllCrossLanguageTest`,
gives `Tests run: 134, Failures: 0, Errors: 0, Skipped: 0`.
Full suite with the fix, `mvn --toolchains ... test`, single threaded:
`Tests run: 2208, Failures: 0, Errors: 0, Skipped: 0`.
--
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]
