Skip to content

docs(linalg): document the SIMD conversion panics and fix the AVX2 kernel's algorithm name - #8865

Open
LuciferYang wants to merge 1 commit into
lance-format:mainfrom
LuciferYang:docs/linalg-simd-contracts
Open

docs(linalg): document the SIMD conversion panics and fix the AVX2 kernel's algorithm name#8865
LuciferYang wants to merge 1 commit into
lance-format:mainfrom
LuciferYang:docs/linalg-simd-contracts

Conversation

@LuciferYang

Copy link
Copy Markdown
Contributor

What this changes

Adds a # Panics section to the six From<&[T]> conversions in rust/lance-linalg/src/simd/, and corrects one doc line in hamming.rs that names an algorithm the code does not use. Comment-only, 31 added lines and 1 removed. Addresses #8861.

Why

#8593 gave every slice-to-register conversion an always-on assert!, so a short slice panics rather than reading out of bounds. None of the six said so, and From is the conversion a caller reaches for without reading the body: the signature is infallible and nothing in the rendered docs warned that f32x8::from(&slice[..]) aborts the thread. The six are f32x8, f32x16, f64x4, f64x8, i32x8 and u8x16, requiring 8, 16, 4, 8, 8 and 16 elements. The From<&[T; N]> impls beside them take an array reference and cannot be short, so they are left alone.

Separately, hamming.rs described the AVX2 kernel as "Harley-Seal / PSHUFB method". The body builds a nibble popcount table, looks both nibbles up with two _mm256_shuffle_epi8 calls, and sums bytes with _mm256_sad_epu8. Harley-Seal is a carry-save-adder tree that folds several words together before any popcount runs, and there is none here: each 256-bit chunk is popcounted on its own. A grep for Harley-Seal across the crate now returns nothing, so that line was the only occurrence.

Relationship to #8862

#8862 covers the same ground and was opened first. I have left review notes there rather than duplicating it, and this PR is the alternative if the placement question below is easier to take as a diff than as a comment.

The difference that matters is where the section goes. #8862 puts # Panics on the impl block; this puts it on fn from. I rendered both. On the impl block the heading attaches to the impl header and lands outside impl-items, so expanding from on the struct page shows nothing:

<h3 class="code-header">impl From<&[f32]> for f32x8</h3>
<div class="docblock"><h4 id="panics-1">§Panics</h4>...

On the method it attaches to from, one heading level lower, with a summary line above it:

<section id="method.from" class="method trait-impl">
  <h4 class="code-header">fn from(value: &[f32]) -> Self</h4>
</section></summary><div class="docblock">
  <p>Loads the first 8 elements of <code>value</code>.</p>
  <h5 id="panics-1">§Panics</h5>...

Both are valid rustdoc and neither warns under -D warnings.

Test plan

Comment-only, so there is nothing to test beyond the doc build.

  • RUSTDOCFLAGS="-D warnings" cargo doc -p lance-linalg --no-deps: clean, and all six sentences appear in the generated HTML under their from method
  • cargo fmt --all -- --check and cargo clippy -p lance-linalg --all-targets -- -D warnings: clean
  • cargo test --profile ci -p lance-linalg --lib: 389 passed, 1 ignored, unchanged

Not included, so it does not get lost: dot_u8.rs, l2_u8.rs and cosine_u8.rs have zero # Panics and zero # Safety between them despite the same always-on checks and unsafe kernels whose raw loads depend on the lengths matching. That wants one change across all three files, and it has to follow whichever cosine length-contract PR lands.

@github-actions github-actions Bot added A-index Vector index, linalg, tokenizer documentation Improvements or additions to documentation labels Aug 29, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 29, 2026
@LuciferYang
LuciferYang force-pushed the docs/linalg-simd-contracts branch from 05b2d58 to 6f84c88 Compare August 29, 2026 16:18
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 29, 2026

@lance-gatekeeper lance-gatekeeper Bot 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.

Gate recommendation: approve.

After the rebase, the six minimum lengths still match the established assertions and render under their from methods; the AVX2 description still matches the PSHUFB nibble-lookup kernel. Open PR #8862 remains an equivalent duplicate, so only one should land.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-index Vector index, linalg, tokenizer documentation Improvements or additions to documentation K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant