Skip to content

Add a Normalized encoding for tensor columns - #9138

Open
connortsui20 wants to merge 2 commits into
developfrom
ct/l2-denorm-encoding
Open

Add a Normalized encoding for tensor columns#9138
connortsui20 wants to merge 2 commits into
developfrom
ct/l2-denorm-encoding

Conversation

@connortsui20

@connortsui20 connortsui20 commented Aug 2, 2026

Copy link
Copy Markdown
Member

Adds Normalized, a norm-split array encoding for tensor columns with normalized and norms children.

Tracking issue: #9129

Normalized owns the physical decomposition and validates the invariant between its children. Scalar functions remain operations over any well-typed input, while the encoding can guarantee that its direction child is unit-normalized and that its stored norm is valid.

Changes

  • Renames L2Denorm and encodings::l2_denorm to Normalized and encodings::normalized.
  • Normalized::try_new validates exact normalized rows. Normalized::new_unchecked admits structurally valid lossy children whose stored norms remain authoritative.
  • Slice and filter push down into both children, preserving the encoding.
  • NormalizedScheme cascades compression into both children.
  • Cosine similarity, inner product, and L2 norm keep their read-through paths for Normalized arrays.

API changes

Before After
scalar_fns::l2_denorm::* encodings::normalized::*
L2Denorm::try_new_array(n, s, ctx) Normalized::try_new(n, s, ctx)
unsafe L2Denorm::new_array_unchecked(n, s) unsafe Normalized::new_unchecked(n, s)
L2Denorm::new() removed
session.scalar_fns().register(L2Denorm) session.arrays().register(Normalized), unconditional
ScalarFnArrayPlugin::new(L2Denorm) removed

The array ID is now vortex.tensor.normalized; the serialized child metadata layout is unchanged. NormalizedScheme remains out of ALL_SCHEMES.

@connortsui20
connortsui20 force-pushed the ct/l2-denorm-encoding branch from 80daa53 to 63e459b Compare August 2, 2026 18:03
@claude claude Bot added the changelog/break A breaking API change label Aug 2, 2026
@connortsui20
connortsui20 marked this pull request as ready for review August 3, 2026 15:11
@connortsui20

Copy link
Copy Markdown
Member Author

There are things I need to touch up here but would appreciate review on the goal of this PR (making this scalar function an encoding instead)

@connortsui20
connortsui20 force-pushed the ct/l2-denorm-encoding branch 2 times, most recently from d62d2a9 to 8598018 Compare August 3, 2026 15:29
@codspeed-hq

codspeed-hq Bot commented Aug 3, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 10.74%

⚡ 1 improved benchmark
✅ 1841 untouched benchmarks
⏩ 44 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation compress_fsst[(1000, 64, 8)] 1.2 ms 1 ms +10.74%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing ct/l2-denorm-encoding (a562e60) with develop (efa73f3)

Open in CodSpeed

Footnotes

  1. 44 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Comment on lines +286 to +287
&ConstantArray::new(normalized, 1).into_array(),
&ConstantArray::new(norms, 1).into_array(),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happened to the conversation about having a special class of scalars that are just length-1 arrays?

@connortsui20
connortsui20 force-pushed the ct/l2-denorm-encoding branch from 8598018 to 780d4d0 Compare August 3, 2026 15:42
gatesn
gatesn previously requested changes Aug 3, 2026

@gatesn gatesn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm

@gatesn
gatesn dismissed their stale review August 3, 2026 18:33

Discussed, I think if we store unitvec + norms, then yes we could make this an encoding.

We could also make this an extension type with a denorm scalar function. I'm not sure where we landed on scalar encodings being modeled as encodings vs scalar functions

@connortsui20 connortsui20 changed the title Make L2Denorm an array encoding instead of a scalar function Add a Normalized encoding for tensor columns Aug 3, 2026
@connortsui20
connortsui20 requested a review from gatesn August 3, 2026 19:11
@connortsui20
connortsui20 force-pushed the ct/l2-denorm-encoding branch from a732ded to c2288dc Compare August 3, 2026 21:18
claude and others added 2 commits August 4, 2026 14:48
`L2Denorm` was registered as a `ScalarFnVTable`, but it never behaved like
one. Its constructor took an `ExecutionCtx` and scanned both children to
enforce a unit-norm invariant, `L2Norm` read its stored norms instead of
recomputing, `CosineSimilarity` and `InnerProduct` reached into its physical
children, and the compressor scheme named it as a produced encoding. Those
are all properties of a physical decomposition, not of an operation over
arbitrary well-typed values.

Moves it to `vortex-tensor/src/encodings/l2_denorm/` as a real `VTable` with
two slots (`normalized`, `norms`). Structural validation runs on construction
and on deserialization, `try_new` additionally scans for the exact unit-norm
invariant, and `try_new_trusted` skips that scan for lossy normalized
children whose stored norms stay authoritative. Neither constructor is
`unsafe`, since violating the contract produces wrong answers rather than
undefined behavior.

The encoding keeps the `vortex.tensor.l2_denorm` array ID and the same
two-field metadata message, so the wire format is unchanged.

Slice and filter now push down into both children through `reduce_parent`.
The generic `ScalarFnArray` filter rule only fired when at most one child was
non-constant, which for this encoding was almost never.

Also makes `L2DenormScheme` cascade its two children like `TemporalScheme`
does, which lets both `HACK TO SUPPORT L2 DENORMALIZATION` special cases come
out of `CascadingCompressor`. The scheme now competes on measured size like
every other scheme.

Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20
connortsui20 force-pushed the ct/l2-denorm-encoding branch from c2288dc to a562e60 Compare August 4, 2026 18:57
@robert3005

Copy link
Copy Markdown
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/break A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants