Train ALP-RD dictionary on a stratified sample#308
Merged
Conversation
ALP-RD built its left-parts dictionary from the first SAMPLE_SIZE physical rows (findBestDictionaryF64/F32 iterated values[0..sampleLen)). The cascade measures ALP-RD's cost on its own stratified sample, so a head-only dictionary could look cheap in the competition yet flood the tail with exceptions on the full re-encode when the leading rows are unrepresentative (sorted or clustered floats) — a size regression newly reachable now that ALP-RD is a top-level cascade candidate (#304 review). Train on a stratified sample spanning the whole array instead, matching AlpEncodingEncoder.findExponentsF64. Also harden AlpRdCascadeSelectionIntegrationTest: besides asserting the categorical ALP-RD win (decided on a 4096-row sample), assert the file compresses below raw F64, so the test can't pass on a degenerate "selected but didn't beat raw" outcome. nyc-311 unchanged at 1644.25 MB (its leading rows were already representative); 408 round-trip property tests, 16 ALP-RD unit tests, and 217 interop tests pass; full verify green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
Follow-up to #304 (PR #307), from that PR's review. Two fixes:
1. ALP-RD sampled its dictionary from the head, not a stratified sample.
findBestDictionaryF64/F32trained the left-parts dictionary fromvalues[0 .. SAMPLE_SIZE)— the first physical rows. But the cascade measures ALP-RD's cost on its own stratified sample, so a head-only dictionary can look cheap in the competition and then flood the tail with exceptions on the full re-encode when the leading rows are unrepresentative (sorted or clustered floats). That's a size-regression vector newly reachable now that ALP-RD is a top-level cascade candidate.AlpEncodingEncoder.findExponentsF64already strides across the whole array to avoid exactly this; ALP-RD now does the same.2. Hardened the selection test.
AlpRdCascadeSelectionIntegrationTestasserted only the categoricalusedEncodings().contains("vortex.alprd")— a winner decided on a 4096-row sample, with no size margin. It now also asserts the file compresses below raw F64, so it can't pass on a degenerate "selected but didn't actually beat raw" outcome.Impact
Correctness-neutral (ALP-RD is lossless regardless of dictionary contents — exceptions cover misses); this is a size-robustness fix for adversarially-ordered float columns.
./mvnw verifygreen.🤖 Generated with Claude Code