Fixes for get partition boundaries #475
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
getPartitionBoundaries(...) in all the quantile sketches for small values of N.
- This is a large number of changes. - The problem detected by the Druid team is fixed, so now the "getPartitionBoundaries" works for input streams that are larger than Integer.MAX_VALUE. - This fix applies to both the KllItemsSketch and the classic ItemsSketch. These are the only two sketches, for now, that will support the "getPartitionBoundaries" functionality. This is enforced via a new "PartitioningFeature" API interface. - In addition, there is new "partitions" package that solves the problem of limited accuracy of our quantiles sketches when being asked to partition very large input streams. This package can partition very large streams of almost unlimited size with very small variation in the resulting partition sizes. I have tested this with streams as large as 30E12 elements. - I have reduced code duplication in a number of places. Specifically, All the quantile sketch sorted view classes use only 3 iterator implementations, which are for float, double and generic. Further consolidation of classes can be done across the sorted view classes themselves, but that will have to be done later. - Javadocs have been improved in a number of places and I have fixed spelling errors when I see them.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/test/java/org/apache/datasketches/partitions/ItemsSketchFillRequestLongAsString.java
Fixed
Show resolved
Hide resolved
src/test/java/org/apache/datasketches/partitions/ItemsSketchFillRequestLongAsString.java
Fixed
Show resolved
Hide resolved
src/test/java/org/apache/datasketches/partitions/KllItemsSketchFillRequestLongAsString.java
Fixed
Show resolved
Hide resolved
src/test/java/org/apache/datasketches/partitions/KllItemsSketchFillRequestLongAsString.java
Fixed
Show resolved
Hide resolved
src/test/java/org/apache/datasketches/quantilescommon/LongsAsOrderableStrings.java
Fixed
Show resolved
Hide resolved
|
This is the final review before I start the release process for Java - 5.0.0 |
1. providing a main(...) 2. Initiate via TestNG 3. Direct progammatic access. I also split out the reporting method as it was duplicate code.
jmalkin
reviewed
Nov 28, 2023
src/main/java/org/apache/datasketches/kll/KllDoublesSketchSortedView.java
Show resolved
Hide resolved
src/main/java/org/apache/datasketches/kll/KllItemsSketchSortedView.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/datasketches/kll/KllItemsSketchSortedView.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/datasketches/partitions/Partitioner.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/datasketches/quantilescommon/Stack.java
Outdated
Show resolved
Hide resolved
src/test/java/org/apache/datasketches/partitions/ClassicPartitionsTest.java
Fixed
Show resolved
Hide resolved
src/test/java/org/apache/datasketches/partitions/ClassicPartitionsTest.java
Fixed
Show resolved
Hide resolved
src/test/java/org/apache/datasketches/partitions/ClassicPartitionsTest.java
Fixed
Show resolved
Hide resolved
src/test/java/org/apache/datasketches/partitions/ClassicPartitionsTest.java
Fixed
Show resolved
Hide resolved
src/test/java/org/apache/datasketches/partitions/ClassicPartitionsTest.java
Fixed
Show resolved
Hide resolved
AlexanderSaydakov
approved these changes
Nov 30, 2023
The code included here does work fine for moderate sized partitioning tasks. As an example, using the test code in the test branch with the partitioning task of splitting a data set of 1 billion items into 324 partitions of size 3M items completed in under 3 minutes, which was performed on a single CPU. For much larger partitioning tasks, it is recommended that this code be leveraged into a parallelized systems environment. I made some minor tweaks to the test code examples.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This applies the same fix that eliminates duplicate entries when using getPartitionBoundaries(...) for small values of N, in all the quantile sketches.