Skip to content

Unify vector-like structs #214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JoeyBF wants to merge 32 commits into
base: master
Choose a base branch
from
Open

Unify vector-like structs #214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
e42e801
WIP with const alignment and transparent repr
JoeyBF Nov 30, 2025
ed71aca
Simple docs
JoeyBF Nov 30, 2025
5698120
Factor out `FqVector::is_empty`
JoeyBF Nov 26, 2025
ba9d9f0
Factor out `FpVector::prime`
JoeyBF Nov 26, 2025
63233aa
Factor out `FqVector::slice`
JoeyBF Nov 26, 2025
aeed08f
Factor out `FqVector::slice_mut`
JoeyBF Nov 26, 2025
55e6c4d
Factor out `FqVector::entry`
JoeyBF Nov 26, 2025
c2c1252
Factor out `FqVector::set_entry`
JoeyBF Nov 26, 2025
dc93541
Factor out `FqVector::add_basis_element`
JoeyBF Nov 26, 2025
3152201
Move limb methods to `FqVectorBase`
JoeyBF Nov 26, 2025
8040c20
Factor out `FqVector::set_to_zero`
JoeyBF Nov 26, 2025
db05728
Factor out `FqVector::is_zero`
JoeyBF Nov 26, 2025
0f3c789
Factor out `FqSliceMut::reduce_limbs`
JoeyBF Nov 26, 2025
f50a543
Factor out `FqVector::scale`
JoeyBF Nov 26, 2025
80bc55d
Remove unnecessary `.as_slice`s
JoeyBF Nov 26, 2025
417ef3f
Factor out `FqVector::as_slice`
JoeyBF Nov 26, 2025
e341997
Factor out `FqVector::as_slice_mut`
JoeyBF Nov 26, 2025
0a36f21
Factor out `FqVector::iter`
JoeyBF Nov 26, 2025
4e656f1
Factor out `FqVector::iter_nonzero`
JoeyBF Nov 26, 2025
636ee4b
Factor out `Display` impl for vectors
JoeyBF Nov 26, 2025
b07ff73
Implement `From<&'a FqVectorBase>` for `FqSlice<'a>`
JoeyBF Nov 26, 2025
2867ecb
Implement `From<&'a mut FqVectorBase>` for `FqSliceMut<'a>`
JoeyBF Nov 26, 2025
22803a6
Use `Repr` in `fp_wrapper` too
JoeyBF Nov 28, 2025
d416a86
Factor out immutable methods to `FpVectorBase`
JoeyBF Nov 28, 2025
81ad0ae
Factor out mutable methods
JoeyBF Nov 28, 2025
e26d5be
Unify `From` implementations for slice types
JoeyBF Nov 28, 2025
870fe7a
Unify `Display` implementations
JoeyBF Nov 29, 2025
cec5fbb
Extend `Vec`'s `From<&FpVector>` implementation
JoeyBF Nov 30, 2025
2b4061f
Relax requirements of `FqVectorBase` definition
JoeyBF Dec 10, 2025
e55feb7
Make `FpVectorBase` an enum in all feature combinations
JoeyBF Dec 10, 2025
8455b53
Fix lint
JoeyBF Dec 26, 2025
0e27e28
Clean up
JoeyBF Dec 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ext/crates/algebra/src/module/free_module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ impl<const U: bool, A: MuAlgebra<U>> MuFreeModule<U, A> {
let len = self
.algebra()
.dimension_unstable(degree - gen_degree, gen_degree);
v.slice(
v.restrict(
std::cmp::min(v.len(), start),
std::cmp::min(v.len(), start + len),
)
Comment on lines +367 to 370
Copy link

@coderabbitai coderabbitai bot Dec 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find all uses of .slice( in the codebase
rg -n '\.slice\(' --type=rust | head -50

Repository: SpectralSequences/sseq

Length of output: 4511

🏁 Script executed:

# Check the specific file around lines 198 and 367
cat -n ext/crates/algebra/src/module/free_module.rs | sed -n '190,210p'
cat -n ext/crates/algebra/src/module/free_module.rs | sed -n '360,375p'

Repository: SpectralSequences/sseq

Length of output: 1617

🏁 Script executed:

# Find FpSlice definition to check available methods
rg -n 'impl.*FpSlice' --type=rust -A 10 | head -100

Repository: SpectralSequences/sseq

Length of output: 7329

🏁 Script executed:

# Search for the restrict method definition
rg -n 'fn restrict' --type=rust -B 2 -A 5

Repository: SpectralSequences/sseq

Length of output: 1864

🏁 Script executed:

# Search for the definition of .slice( method
rg -n 'fn slice' --type=rust -B 2 -A 5 ext/crates/fp/

Repository: SpectralSequences/sseq

Length of output: 3272

🏁 Script executed:

# Check the full impl blocks for FpSlice to see all methods
rg -n 'impl.*FpSlice|impl.*FqSlice' --type=rust -A 20 ext/crates/fp/src/vector/

Repository: SpectralSequences/sseq

Length of output: 11424

Update line 198 to use the new .restrict() API.

Line 198 should use input.restrict(input_start, input_end) to be consistent with line 367. The FpSlice::slice() method does not exist; the new unified API is FpSlice::restrict().

🤖 Prompt for AI Agents 
ext/crates/algebra/src/module/free_module.rs around lines 367-370 (also update
the use at line ~198): change the old FpSlice::slice(...) call to use the new
FpSlice::restrict(start, end) API; compute start as std::cmp::min(input.len(),
input_start) and end as std::cmp::min(input.len(), input_start + len) and pass
those to input.restrict(start, end) so it mirrors the usage at lines 367–370.

Copy link
Collaborator Author

@JoeyBF JoeyBF Dec 26, 2025

Choose a reason for hiding this comment

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

Is it not already using restrict?

Copy link

@coderabbitai coderabbitai bot Dec 26, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the exact line number where input.slice is used in this file
grep -n 'input.slice' ext/crates/algebra/src/module/free_module.rs

# Also show context around that line
grep -n -C 3 'input.slice' ext/crates/algebra/src/module/free_module.rs

Length of output: 631

@JoeyBF, you're right that line 367 (shown in the diff) is already using restrict() — that change looks good!

However, line 198 still uses the old .slice() method:

let input_slice = input.slice(input_start, input_end);

This should be updated to use .restrict() as well, to match the new unified API used at line 367:

let input_slice = input.restrict(input_start, input_end);

Expand Down
26 changes: 13 additions & 13 deletions ext/crates/fp/src/matrix/matrix_inner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,12 +367,12 @@ impl Matrix {

pub fn row(&self, row: usize) -> FpSlice<'_> {
let limb_range = row_to_limb_range(row, self.stride);
FpSlice::new(self.prime(), &self.data[limb_range], 0, self.columns)
FpSlice::_new(self.prime(), &self.data[limb_range], 0, self.columns)
}

pub fn row_mut(&mut self, row: usize) -> FpSliceMut<'_> {
let limb_range = row_to_limb_range(row, self.stride);
FpSliceMut::new(self.prime(), &mut self.data[limb_range], 0, self.columns)
FpSliceMut::_new(self.prime(), &mut self.data[limb_range], 0, self.columns)
}
}

Expand All @@ -393,7 +393,7 @@ impl Matrix {
.data
.chunks_mut(self.stride)
.take(logical_rows) // Only iterate over logical rows
.map(move |row| FpSliceMut::new(p, row, 0, columns));
.map(move |row| FpSliceMut::_new(p, row, 0, columns));
Either::Right(rows)
}
}
Expand All @@ -412,7 +412,7 @@ impl Matrix {
.data
.maybe_par_chunks_mut(self.stride)
.take(logical_rows) // Only iterate over logical rows
.map(move |row| FpSliceMut::new(p, row, 0, columns));
.map(move |row| FpSliceMut::_new(p, row, 0, columns));
Either::Right(rows)
}
}
Expand Down Expand Up @@ -516,8 +516,8 @@ impl Matrix {
let row1 = unsafe { std::slice::from_raw_parts_mut(ptr.add(i * self.stride), self.stride) };
let row2 = unsafe { std::slice::from_raw_parts_mut(ptr.add(j * self.stride), self.stride) };
(
FpSliceMut::new(self.prime(), row1, 0, self.columns),
FpSliceMut::new(self.prime(), row2, 0, self.columns),
FpSliceMut::_new(self.prime(), row1, 0, self.columns),
FpSliceMut::_new(self.prime(), row2, 0, self.columns),
)
}

Expand Down Expand Up @@ -1263,13 +1263,13 @@ impl<const N: usize> AugmentedMatrix<N> {
let start_idx = self.start[start];
let end_idx = self.end[end];
let limb_range = row_to_limb_range(i, self.stride);
FpSliceMut::new(self.prime(), &mut self.data[limb_range], start_idx, end_idx)
FpSliceMut::_new(self.prime(), &mut self.data[limb_range], start_idx, end_idx)
}

pub fn row_segment(&self, i: usize, start: usize, end: usize) -> FpSlice<'_> {
let start_idx = self.start[start];
let end_idx = self.end[end];
self.row(i).slice(start_idx, end_idx)
self.row(i).restrict(start_idx, end_idx)
}

pub fn into_matrix(self) -> Matrix {
Expand Down Expand Up @@ -1414,7 +1414,7 @@ impl<'a> MatrixSliceMut<'a> {
let end = self.col_end;
(0..self.rows).map(move |row_idx| {
let limb_range = row_to_limb_range(row_idx, self.stride);
FpSlice::new(self.prime(), &self.data[limb_range], start, end)
FpSlice::_new(self.prime(), &self.data[limb_range], start, end)
})
}

Expand All @@ -1429,7 +1429,7 @@ impl<'a> MatrixSliceMut<'a> {
let rows = self
.data
.chunks_mut(self.stride)
.map(move |row| FpSliceMut::new(p, row, start, end));
.map(move |row| FpSliceMut::_new(p, row, start, end));
Either::Right(rows)
}
}
Expand All @@ -1447,14 +1447,14 @@ impl<'a> MatrixSliceMut<'a> {
let rows = self
.data
.maybe_par_chunks_mut(self.stride)
.map(move |row| FpSliceMut::new(p, row, start, end));
.map(move |row| FpSliceMut::_new(p, row, start, end));
Either::Right(rows)
}
}

pub fn row(&mut self, row: usize) -> FpSlice<'_> {
let limb_range = row_to_limb_range(row, self.stride);
FpSlice::new(
FpSlice::_new(
self.prime(),
&self.data[limb_range],
self.col_start,
Expand All @@ -1464,7 +1464,7 @@ impl<'a> MatrixSliceMut<'a> {

pub fn row_mut(&mut self, row: usize) -> FpSliceMut<'_> {
let limb_range = row_to_limb_range(row, self.stride);
FpSliceMut::new(
FpSliceMut::_new(
self.prime(),
&mut self.data[limb_range],
self.col_start,
Expand Down
61 changes: 26 additions & 35 deletions ext/crates/fp/src/vector/fp_wrapper/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,38 +14,36 @@

use itertools::Itertools;

use super::{FqSlice, FqSliceMut, FqVector, FqVectorIterator, FqVectorNonZeroIterator};
use super::{
FqSlice, FqSliceMut, FqVector, FqVectorBase, FqVectorIterator, FqVectorNonZeroIterator, Repr,
ReprMut,
};
use crate::field::Field;

impl<F: Field> FqVector<F> {
pub(super) fn scale_helper(&mut self, c: F::ElementContainer) {
self.scale(self.fq().el(c))
}

impl<const A: bool, R: Repr, F: Field> FqVectorBase<A, R, F> {
pub(super) fn entry_helper(&self, index: usize) -> F::ElementContainer {
self.entry(index).val()
}
}

pub(super) fn set_entry_helper(&mut self, index: usize, value: F::ElementContainer) {
self.set_entry(index, self.fq().el(value))
}

pub(super) fn add_helper(&mut self, other: &Self, c: F::ElementContainer) {
self.add(other, self.fq().el(c))
impl<const A: bool, R: ReprMut, F: Field> FqVectorBase<A, R, F> {
pub(super) fn scale_helper(&mut self, c: F::ElementContainer) {
self.scale(self.fq().el(c))
}

pub(super) fn add_offset_helper(
&mut self,
other: &Self,
c: F::ElementContainer,
offset: usize,
) {
self.add_offset(other, self.fq().el(c), offset)
pub(super) fn set_entry_helper(&mut self, index: usize, value: F::ElementContainer) {
self.set_entry(index, self.fq().el(value))
}

pub(super) fn add_basis_element_helper(&mut self, index: usize, value: F::ElementContainer) {
self.add_basis_element(index, self.fq().el(value))
}
}

impl<F: Field> FqVector<F> {
pub(super) fn add_helper(&mut self, other: &Self, c: F::ElementContainer) {
self.add(other, self.fq().el(c))
}

pub(super) fn copy_from_slice_helper(&mut self, other: &[F::ElementContainer]) {
self.copy_from_slice(&other.iter().map(|x| self.fq().el(x.clone())).collect_vec())
Expand All @@ -71,26 +69,27 @@ impl<F: Field> FqVector<F> {
self.add_carry(other, self.fq().el(c), rest)
}

pub(super) fn add_offset_helper(
&mut self,
other: &Self,
c: F::ElementContainer,
offset: usize,
) {
self.add_offset(other, self.fq().el(c), offset)
}

pub(super) fn first_nonzero_helper(&self) -> Option<(usize, F::ElementContainer)> {
self.first_nonzero().map(|(idx, c)| (idx, c.val()))
}
}

impl<F: Field> FqSlice<'_, F> {
pub(super) fn entry_helper(&self, index: usize) -> F::ElementContainer {
self.entry(index).val()
}

pub(super) fn first_nonzero_helper(&self) -> Option<(usize, F::ElementContainer)> {
self.first_nonzero().map(|(idx, c)| (idx, c.val()))
}
}

impl<F: Field> FqSliceMut<'_, F> {
pub(super) fn scale_helper(&mut self, c: F::ElementContainer) {
self.scale(self.fq().el(c))
}

pub(super) fn add_helper(&mut self, other: FqSlice<F>, c: F::ElementContainer) {
self.add(other, self.fq().el(c))
}
Expand All @@ -104,14 +103,6 @@ impl<F: Field> FqSliceMut<'_, F> {
self.add_offset(other, self.fq().el(c), offset)
}

pub(super) fn set_entry_helper(&mut self, index: usize, value: F::ElementContainer) {
self.set_entry(index, self.fq().el(value))
}

pub(super) fn add_basis_element_helper(&mut self, index: usize, value: F::ElementContainer) {
self.add_basis_element(index, self.fq().el(value))
}

pub(super) fn add_masked_helper(
&mut self,
other: FqSlice<F>,
Expand Down
Loading