Skip to content

Clean up vortex-tensor#7525

Merged
connortsui20 merged 4 commits intodevelopfrom
ct/cleanup-tensor
Apr 20, 2026
Merged

Clean up vortex-tensor#7525
connortsui20 merged 4 commits intodevelopfrom
ct/cleanup-tensor

Conversation

@connortsui20
Copy link
Copy Markdown
Contributor

@connortsui20 connortsui20 commented Apr 19, 2026

Summary

Tracking issue: #7297

Given our fast velocity on this crate, quite a few things slipped through the cracks.

This change cleans up the vortex-tensor crate by clearly defining the abstraction points, fixing a few bugs (the only real bug was a Tensor Display bug), cleaning up some TODOs, and generally raising the quality.

API Changes

The only relevant change is new helper functions

Testing

Just fixed up existing tests.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 19, 2026

Merging this PR will degrade performance by 18.79%

❌ 6 regressed benchmarks
✅ 1152 untouched benchmarks
🆕 5 new benchmarks
⏩ 1462 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation old_alp_prim_test_between[f64, 32768] 284.8 µs 350.7 µs -18.79%
Simulation patched_take_10k_contiguous_patches 228.1 µs 258.2 µs -11.64%
Simulation patched_take_10k_random 240.4 µs 270.4 µs -11.12%
Simulation take_10k_first_chunk_only 225.9 µs 270.9 µs -16.59%
Simulation patched_take_10k_contiguous_not_patches 228.5 µs 258.6 µs -11.62%
Simulation take_10k_dispersed 239.8 µs 284.7 µs -15.77%
🆕 Simulation turboquant_encode_dim768_4bit N/A 53 ms N/A
🆕 Simulation turboquant_encode_dim128_4bit N/A 6.7 ms N/A
🆕 Simulation turboquant_encode_dim1024_4bit N/A 53.5 ms N/A
🆕 Simulation turboquant_encode_dim1024_2bit N/A 48.3 ms N/A
🆕 Simulation turboquant_encode_dim1024_8bit N/A 63.7 ms N/A

Comparing ct/cleanup-tensor (d7f386c) with develop (4135209)2

Open in CodSpeed

Footnotes

  1. 1462 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.

  2. No successful run was found on develop (a6dd776) during the generation of this report, so 4135209 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Comment on lines -76 to -85
/// The input `fsl` must contain non-nullable, unit-norm vectors (already L2-normalized). Null
/// vectors are not supported and must be zeroed out before reaching this function. The rotation
/// and centroid lookup happen in f32.
fn turboquant_quantize_core(
fsl: &FixedSizeListArray,
seed: u64,
bit_width: u8,
num_rounds: u8,
ctx: &mut ExecutionCtx,
) -> VortexResult<QuantizationResult> {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is just moved down

// TODO(connor): FIX THIS!!!
fn scheme_name(&self) -> &'static str {
"vortex.tensor.UNSTABLE.l2_denorm"
"vortex.tensor.l2_denorm"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I forgot that this is a completely separate label than the array IDs, and this is confined to just ID equality checking in the compressor

@connortsui20 connortsui20 marked this pull request as ready for review April 19, 2026 17:32
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
@connortsui20 connortsui20 enabled auto-merge (squash) April 20, 2026 02:55
@connortsui20 connortsui20 merged commit 12457aa into develop Apr 20, 2026
56 of 58 checks passed
@connortsui20 connortsui20 deleted the ct/cleanup-tensor branch April 20, 2026 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants