Store Normalized validity on the array instead of in its children - #9202
Draft
connortsui20 wants to merge 1 commit into
Draft
Store Normalized validity on the array instead of in its children#9202connortsui20 wants to merge 1 commit into
Normalized validity on the array instead of in its children#9202connortsui20 wants to merge 1 commit into
Conversation
Closes #9200 `Normalized` derived its validity by `and`ing its two children's, which made the parent's nullability a function of two independently nullable children. That is what every dtype-widening bug in the encoding came from: the decode paths and the read-through operators each had to get from a child's nullability to the parent's, and each got it wrong in a different case. Nulls now live on the array. Both children are non-nullable, `normalize` zeroes them at null positions, and the array carries a `validity` slot whose presence is what makes its dtype nullable. Neither the decode path nor a read-through operator has to widen a child's dtype any more, and `validity()` is a match on a slot rather than a `Binary(And)` array that has to be built and optimized on every call. ## Fixes - Both children nullable plus a non-unit constant norm panicked with `Tried to create an `ExtensionArray` with an incompatible storage array`: the dtype guard on the constant path caught a nullable norms child paired with a non-nullable normalized child, but not the case where both were nullable, and the multiply then widened the FSL elements. The guard is gone along with the state it guarded against. - `L2Norm`'s read-through asserted exact dtype equality against a `norm_dtype` it took from the parent, so it failed on any column whose nullability came from the `normalized` child. It now reattaches the array's null map to the non-nullable norms child. - `try_new` accepted an all-zero row paired with a non-zero stored norm. That decodes to zeros while `L2Norm` reads the stored norm straight back, so the split it promises is lossless was not. The zero-norm rule is now checked in both directions. - `NormalizedScheme::matches` claimed any `AnyTensor` extension, but `compress` gates on a float element ptype. Since the scheme reports `AlwaysUse`, an `i32` tensor column was claimed and then aborted the whole column's compression instead of falling through to another scheme. `matches` now requires a float element ptype. ## Also - `NormalizedMetadata` is removed. It existed only because the parent's unioned nullability could not say which child was nullable; with non-nullable children, the parent dtype and the child count carry everything, so the array serializes no metadata. - The constant-norms identity path now requires a norm of exactly `1.0`. Skipping the multiply for a merely near-unit norm left `scalar_at` -- which routes every row through that path -- answering differently than a bulk decode of the same column. - `validate_l2_normalized_rows_against_norms` is renamed to `validate_normalized_rows`, and no longer takes validity into account: a zeroed null row satisfies both directions of the zero-norm rule on its own. - Fixes "An `Normalized`" in the eight places the rename left it, and restores the alphabetical order of the 2026-04 edition's `added` list. ## Checks | Check | Result | | --- | --- | | `cargo test -p vortex-tensor` | 180 passed | | `cargo test -p vortex --features unstable_encodings` | 26 passed | | `cargo test -p vortex-file --features unstable_encodings` | 133 passed | | `cargo test -p vortex-btrblocks -p vortex-compressor` | 93 passed, 1 skipped | | `cargo clippy -p vortex-tensor -p vortex -p vortex-bench --all-targets --features vortex/unstable_encodings` | clean | | `cargo clippy -p vortex-tensor --all-targets --all-features` | clean | | `cargo +nightly fmt --all` | clean | | `cargo test --doc -p vortex-tensor -p vortex` | clean | Each of the four functional fixes was confirmed to reproduce before the change, and each regression test was confirmed to fail when its fix alone is reverted. Not run: workspace-wide `cargo clippy --all-features` (the `vortex-cuda` build script needs to download nvCOMP, which this sandbox cannot reach), Python/Java bindings, docs. Signed-off-by: Connor Tsui <connor@spiraldb.com>
connortsui20
marked this pull request as draft
August 5, 2026 16:17
Merging this PR will improve performance by 19.92%
Performance Changes
Tip Curious why this is faster? Comment Comparing 
Footnotes
|
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
2 participants
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.
Rationale for this change
Normalizedencoding existing issues #9200Normalizedderived its validity byanding its two children's, which made the array's nullability a function of two independently nullable children. Every dtype-widening defect in #9200 follows from that shape: the decode paths and the read-through operators each had to get from a child's nullability to the parent's, and each got it wrong in a different case.What changes are included in this PR?
Nulls move onto the array. Both children become non-nullable,
normalizezeroes them at null positions, and avalidityslot is what makes the array's dtype nullable, so nothing widens a child's dtype any more. That removes the panic on doubly-nullable children and letsL2Norm's read-through reattach the array's null map rather than assert on a mismatched dtype. The other three fixes are independent: the zero-norm rule is checked in both directions,NormalizedScheme::matchesrequires a float element ptype so ani32tensor falls through instead of aborting compression, and the constant-norms identity path requires a norm of exactly1.0soscalar_atcannot disagree with a bulk decode. Two consequences worth review:validate_normalized_rowsno longer consults validity, sotry_newrejects raw children paired with a mask unless their null positions are already zeroed, and the validity slot is passed through uncompressed likeFixedSizeListArrayvalidity, which is a small size regression for nullable columns.What APIs are changed? Are there any user-facing changes?
All of this sits behind
unstable_encodings, andvortex.tensor.normalizedbelongs to the unstable 2026-04 edition, so the serialized layout change carries no compatibility obligation.try_newandnew_uncheckedtake aValidityand require non-nullable children,NormalizedMetadatais removed because the parent dtype and child count now carry everything it held, andvalidate_l2_normalized_rows_against_normsis renamed tovalidate_normalized_rows.🤖 Generated with Claude Code
https://claude.ai/code/session_01EC8wGdby4GdyxR5J2YrnVi