leerho commented on code in PR #734:
URL: https://github.com/apache/datasketches-java/pull/734#discussion_r3228341252
##########
src/main/java/org/apache/datasketches/req/ReqSerDe.java:
##########
@@ -205,7 +205,7 @@ static final Compactor extractCompactor(final
PositionalSegment posSeg, final bo
final float[] arr = new float[count];
posSeg.getFloatArray(arr, 0, count);
float minItem = Float.MAX_VALUE;
- float maxItem = Float.MIN_VALUE;
+ float maxItem = -Float.MAX_VALUE;
Review Comment:
What you found here is obviously incorrect, but neither is your solution.
Internally, for the Classic Quantiles, KLL and REQ, the min and max values are
initialized to NaN. and the first incoming value that is not NaN, sets both the
min and max values. This allows input values of +/- infinity, which your
solution would not recognize. I would argue that you should use a similar
solution here, for consistency if nothing else.
I would also suggest you create a test that uses +/- Infinities, just to
confirm that it works.
--
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]
