Direction is right: this is a physical decomposition, not a scalar function. Two real wins — slice/filter push-down now always fires, and both HACK TO SUPPORT L2 DENORMALIZATION cases leave CascadingCompressor. I verified the hack removal is safe: L2Denorm was the only scheme producing an AnyScalarFn.
Items 1, 2, and 4 also reproduce on develop — they're pre-existing, not regressions from this PR. But this PR rewrites each of those code paths, so it'd be nice to fix them here rather than leave them behind the rename.
1. Panic on nullable constant norms — execute.rs:100-113
The new dtype guard blocks nullable-norms + non-nullable-child, but not both nullable. norm.clone() stays nullable, the multiply widens the FSL elements, and tensor storage elements must be non-nullable:
Unable to create `ExtensionArray`:
Other error: Tried to create an `ExtensionArray` with an incompatible storage array
Build scale from a non-nullable copy of the scalar. A test alongside nullable_constant_norms_widen_the_decoded_dtype with the nullable-normalized-child variant would pin both halves.
2. L2Norm errors on a valid array — l2_norm.rs:131-136
norm_dtype takes its nullability from the parent (the union of both children), but the read-through returns the norms child unchanged and asserts exact dtype equality. When the parent's nullability comes from the normalized child — exactly the shape serde_round_trip_preserves_normalized_nullability builds — it fails:
Assertion failed error: norms.dtype() != &norm_dtype:
Primitive(F64, NonNullable) != Primitive(F64, Nullable)
Widen the norms to norm_dtype instead of erroring.
3. try_new accepts a zero row paired with a non-zero norm — validate.rs:158-175
Only the stored_norm == 0.0 ⟹ all-zero row direction is checked. normalized = [0.0, 0.0] with norms = [5.0] is accepted by the checked constructor, decodes to [0.0, 0.0], but L2Norm read-through returns 5.0. try_new is documented as the constructor that makes the split lossless, and this state isn't. Add the symmetric vortex_ensure! plus an rstest case mirroring nonzero_row_with_zero_norm.
4. NormalizedScheme::matches accepts non-float tensors — compress.rs:57-63
matches only checks ext_dtype().is::<AnyTensor>(), but compress calls validate_tensor_float_input. An i32 tensor column aborts the entire compression rather than falling through to another scheme:
integer tensor compression ERRORED: Other error: expected a float element dtype, got i32
matches should also require a float element ptype. This is what stands between the scheme and ALL_SCHEMES, which is the stated goal of making it compete on measured size like every other scheme.
5. Smaller items
- Grammar from the rename: "An [
Normalized]" appears 8 times — array.rs:37,80,93,128, compress.rs:140,195, mod.rs:6, cosine_similarity.rs:120.
validate_l2_normalized_rows_against_norms is the one public name left on the old naming scheme.
- The edition
added list is no longer alphabetical: inner_product, normalized, l2_norm.
- Commit 1's message says "Neither constructor is
unsafe", which the final code contradicts. Keeping unsafe fn new_unchecked is right — it matches FixedSizeListArray::new_unchecked — so just reword on squash.
scalar_at inherits the constant-norms tolerance: a row whose norm is within unit_norm_tolerance of 1.0 comes back unscaled, while the bulk execute path multiplies exactly. The gap is ~ε relative, but it's a per-row semantic the bulk path doesn't have.
Checks run
| Check |
Result |
cargo nextest run -p vortex-tensor |
166 passed |
cargo nextest run -p vortex-compressor -p vortex-btrblocks |
89 passed, 1 skipped |
cargo clippy -p vortex-tensor -p vortex-compressor --all-targets --all-features |
clean |
cargo +nightly fmt --all -- --check |
clean |
Findings 1–4 were each confirmed with throwaway probe tests in a scratch worktree; 1, 2, and 4 were re-run against develop to confirm they pre-date this PR. Not run: workspace-wide tests, Python/Java bindings, docs.
Originally posted by @robert3005 in #9138 (comment)
Direction is right: this is a physical decomposition, not a scalar function. Two real wins — slice/filter push-down now always fires, and both
HACK TO SUPPORT L2 DENORMALIZATIONcases leaveCascadingCompressor. I verified the hack removal is safe:L2Denormwas the only scheme producing anAnyScalarFn.Items 1, 2, and 4 also reproduce on
develop— they're pre-existing, not regressions from this PR. But this PR rewrites each of those code paths, so it'd be nice to fix them here rather than leave them behind the rename.1. Panic on nullable constant norms —
execute.rs:100-113The new dtype guard blocks nullable-norms + non-nullable-child, but not both nullable.
norm.clone()stays nullable, the multiply widens the FSL elements, and tensor storage elements must be non-nullable:Build
scalefrom a non-nullable copy of the scalar. A test alongsidenullable_constant_norms_widen_the_decoded_dtypewith the nullable-normalized-child variant would pin both halves.2.
L2Normerrors on a valid array —l2_norm.rs:131-136norm_dtypetakes its nullability from the parent (the union of both children), but the read-through returns thenormschild unchanged and asserts exact dtype equality. When the parent's nullability comes from thenormalizedchild — exactly the shapeserde_round_trip_preserves_normalized_nullabilitybuilds — it fails:Widen the norms to
norm_dtypeinstead of erroring.3.
try_newaccepts a zero row paired with a non-zero norm —validate.rs:158-175Only the
stored_norm == 0.0 ⟹ all-zero rowdirection is checked.normalized = [0.0, 0.0]withnorms = [5.0]is accepted by the checked constructor, decodes to[0.0, 0.0], butL2Normread-through returns5.0.try_newis documented as the constructor that makes the split lossless, and this state isn't. Add the symmetricvortex_ensure!plus an rstest case mirroringnonzero_row_with_zero_norm.4.
NormalizedScheme::matchesaccepts non-float tensors —compress.rs:57-63matchesonly checksext_dtype().is::<AnyTensor>(), butcompresscallsvalidate_tensor_float_input. Ani32tensor column aborts the entire compression rather than falling through to another scheme:matchesshould also require a float element ptype. This is what stands between the scheme andALL_SCHEMES, which is the stated goal of making it compete on measured size like every other scheme.5. Smaller items
Normalized]" appears 8 times —array.rs:37,80,93,128,compress.rs:140,195,mod.rs:6,cosine_similarity.rs:120.validate_l2_normalized_rows_against_normsis the one public name left on the old naming scheme.addedlist is no longer alphabetical:inner_product,normalized,l2_norm.unsafe", which the final code contradicts. Keepingunsafe fn new_uncheckedis right — it matchesFixedSizeListArray::new_unchecked— so just reword on squash.scalar_atinherits the constant-norms tolerance: a row whose norm is withinunit_norm_toleranceof 1.0 comes back unscaled, while the bulkexecutepath multiplies exactly. The gap is ~ε relative, but it's a per-row semantic the bulk path doesn't have.Checks run
cargo nextest run -p vortex-tensorcargo nextest run -p vortex-compressor -p vortex-btrblockscargo clippy -p vortex-tensor -p vortex-compressor --all-targets --all-featurescargo +nightly fmt --all -- --checkFindings 1–4 were each confirmed with throwaway probe tests in a scratch worktree; 1, 2, and 4 were re-run against
developto confirm they pre-date this PR. Not run: workspace-wide tests, Python/Java bindings, docs.Originally posted by @robert3005 in #9138 (comment)