Skip to content
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

Jump to bottom

Optionally borrow polynomial coefficients #237

Merged
merged 2 commits into from
Oct 29, 2024
Merged

Optionally borrow polynomial coefficients #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ed2445
perf(polynomial): Optionally borrow coefficients
jan-ferdinand Oct 16, 2024
65914d5
refactor(polynomial)!: Make `coefficients` private
jan-ferdinand Oct 17, 2024
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
6 changes: 5 additions & 1 deletion twenty-first/benches/formal_power_series_inverse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ fn fpsi(c: &mut Criterion) {

let id = BenchmarkId::new("fpsi", format!("2^{log2_degree}"));
group.bench_function(id, |b| {
b.iter(|| polynomial.formal_power_series_inverse_newton(1 << (log2_degree + 1)))
b.iter(|| {
let precision = 1 << (log2_degree + 1);
let polynomial = polynomial.clone();
polynomial.formal_power_series_inverse_newton(precision)
})
});
}
group.finish();
Expand Down
169 changes: 81 additions & 88 deletions twenty-first/src/math/bfield_codec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,14 +375,25 @@ impl<T: BFieldCodec> BFieldCodec for Vec<T> {

#[derive(Debug, Error)]
pub enum PolynomialBFieldCodecError {
/// A polynomial with leading zero-coefficients, corresponding to an encoding
/// with trailing zeros, is a waste of space. More importantly, it allows for
/// non-canonical encodings of the same underlying polynomial, which is
/// confusing and can lead to errors.
///
/// A note on “leading” vs “trailing”:
/// The polynomial 0·x³ + 2x² + 42 has a spuriuous _leading_ zero, which
/// translates into a spurious _trailing_ zero in its encoding.
#[error("trailing zeros in polynomial-encoding")]
TrailingZerosInPolynomialEncoding,

#[error(transparent)]
Other(#[from] BFieldCodecError),
}

impl<T: BFieldCodec + FiniteField> BFieldCodec for Polynomial<T> {
impl<T> BFieldCodec for Polynomial<'_, T>
where
T: BFieldCodec + FiniteField + 'static,
{
type Error = PolynomialBFieldCodecError;

fn decode(sequence: &[BFieldElement]) -> Result<Box<Self>, Self::Error> {
Expand Down Expand Up @@ -414,21 +425,10 @@ impl<T: BFieldCodec + FiniteField> BFieldCodec for Polynomial<T> {
}

fn encode(&self) -> Vec<BFieldElement> {
let mut normalized_self = self.clone();

// It's critical to normalize the polynomial, i.e. remove trailing zeros from the
// coefficients list, to make the encoding non-degenerate such that a polynomial maps to a
// unique encoding. Length of the coefficients field is prepended to the encoding to make
// the encoding consistent with a derived implementation, with the only difference that
// trailing zeros in the encoding is not allowed.
normalized_self.normalize();
let coefficients_encoded = normalized_self.coefficients.encode();

[
bfe_vec!(coefficients_encoded.len() as u64),
coefficients_encoded,
]
.concat()
// The length of the coefficients field is prepended to the encoding to make the
// encoding consistent with a hypothetical derived implementation.
let coefficients_encoded = self.coefficients().to_owned().encode();
[bfe_vec!(coefficients_encoded.len()), coefficients_encoded].concat()
}

fn static_length() -> Option<usize> {
Expand Down Expand Up @@ -685,8 +685,8 @@ mod tests {
test_case! { fn vec_of_vec_of_bfieldelement for Vec<Vec<BFieldElement>>: None }
test_case! { fn vec_of_vec_of_xfieldelement for Vec<Vec<XFieldElement>>: None }
test_case! { fn vec_of_vec_of_digest for Vec<Vec<Digest>>: None }
test_case! { fn poly_bfe for Polynomial<BFieldElement>: None }
test_case! { fn poly_xfe for Polynomial<XFieldElement>: None }
test_case! { fn poly_bfe for Polynomial<'static, BFieldElement>: None }
test_case! { fn poly_xfe for Polynomial<'static, XFieldElement>: None }
test_case! { fn tuples_static_static_size_0 for (Digest, u128): Some(9) }
test_case! { fn tuples_static_static_size_1 for (Digest, u64): Some(7) }
test_case! { fn tuples_static_static_size_2 for (BFieldElement, BFieldElement): Some(2) }
Expand Down Expand Up @@ -717,78 +717,68 @@ mod tests {

#[test]
fn leading_zero_coefficient_have_no_effect_on_encoding_empty_poly_bfe() {
let poly_no_leading_zeros: Polynomial<BFieldElement> = Polynomial::new(vec![]);
let encoded = poly_no_leading_zeros.encode();

let mut poly_w_leading_zeros: Polynomial<BFieldElement> = Polynomial::new(vec![]);
poly_w_leading_zeros.coefficients.push(bfe!(0));
let poly_w_leading_zeros_encoded = poly_w_leading_zeros.encode();

assert_eq!(encoded, poly_w_leading_zeros_encoded);
let empty_poly = Polynomial::<BFieldElement>::new(vec![]);
assert_eq!(empty_poly.encode(), Polynomial::new(bfe_vec![0]).encode());
}

#[test]
fn leading_zero_coefficients_have_no_effect_on_encoding_empty_poly_xfe() {
let poly_no_trailing_zeros: Polynomial<XFieldElement> = Polynomial::new(vec![]);
let encoded = poly_no_trailing_zeros.encode();

let mut poly_w_leading_zeros: Polynomial<XFieldElement> = Polynomial::new(vec![]);
poly_w_leading_zeros.coefficients.push(xfe!(0));
let poly_w_leading_zeros_encoded = poly_w_leading_zeros.encode();

assert_eq!(encoded, poly_w_leading_zeros_encoded);
let empty_poly = Polynomial::<XFieldElement>::new(vec![]);
assert_eq!(empty_poly.encode(), Polynomial::new(xfe_vec![0]).encode());
}

#[proptest]
fn leading_zero_coefficients_have_no_effect_on_encoding_poly_bfe_pbt(
mut polynomial: Polynomial<BFieldElement>,
polynomial: Polynomial<'static, BFieldElement>,
#[strategy(0usize..30)] num_leading_zeros: usize,
) {
let encoded = polynomial.encode();
polynomial
.coefficients
.extend(vec![BFieldElement::ZERO; num_leading_zeros]);
let poly_w_leading_zeros_encoded = polynomial.encode();
prop_assert_eq!(encoded, poly_w_leading_zeros_encoded);

let mut coefficients = polynomial.into_coefficients();
coefficients.extend(vec![BFieldElement::ZERO; num_leading_zeros]);
let poly_w_leading_zeros = Polynomial::new(coefficients);

prop_assert_eq!(encoded, poly_w_leading_zeros.encode());
}

#[proptest]
fn leading_zero_coefficients_have_no_effect_on_encoding_poly_xfe_pbt(
mut polynomial: Polynomial<XFieldElement>,
polynomial: Polynomial<'static, XFieldElement>,
#[strategy(0usize..30)] num_leading_zeros: usize,
) {
let encoded = polynomial.encode();
polynomial
.coefficients
.extend(vec![XFieldElement::ZERO; num_leading_zeros]);
let poly_w_leading_zeros_encoded = polynomial.encode();
prop_assert_eq!(encoded, poly_w_leading_zeros_encoded);

let mut coefficients = polynomial.into_coefficients();
coefficients.extend(vec![XFieldElement::ZERO; num_leading_zeros]);
let poly_w_leading_zeros = Polynomial::new(coefficients);

prop_assert_eq!(encoded, poly_w_leading_zeros.encode());
}

fn disallow_trailing_zeros_in_poly_encoding_prop<T: FiniteField + BFieldCodec>(
mut polynomial: Polynomial<T>,
leading_coefficient: T,
fn disallow_trailing_zeros_in_poly_encoding_prop<FF>(
polynomial: Polynomial<FF>,
leading_coefficient: FF,
num_leading_zeros: usize,
) -> TestCaseResult {
polynomial.coefficients.push(leading_coefficient);
let vec_encoding = polynomial.coefficients.encode();
let poly_encoding = polynomial.encode();
) -> TestCaseResult
where
FF: FiniteField + BFieldCodec + 'static,
{
let mut polynomial_coefficients = polynomial.into_coefficients();
polynomial_coefficients.push(leading_coefficient);
let actual_polynomial = Polynomial::new(polynomial_coefficients.clone());
let vec_encoding = actual_polynomial.coefficients().to_vec().encode();
let poly_encoding = actual_polynomial.encode();
prop_assert_eq!(
[bfe_vec!(vec_encoding.len() as u64), vec_encoding].concat(),
[bfe_vec!(vec_encoding.len()), vec_encoding].concat(),
poly_encoding,
"This test expects similarity of Vec and Polynomial encoding"
);

polynomial
.coefficients
.extend(vec![T::zero(); num_leading_zeros]);
let vec_encoding_with_trailing_zeros = polynomial.coefficients.encode();
let poly_encoding_with_trailing_zeros = [
bfe_vec!(vec_encoding_with_trailing_zeros.len() as u64),
vec_encoding_with_trailing_zeros,
]
.concat();
let decoding_result = Polynomial::<T>::decode(&poly_encoding_with_trailing_zeros);
polynomial_coefficients.extend(vec![FF::ZERO; num_leading_zeros]);
let coefficient_encoding = polynomial_coefficients.encode();
let poly_encoding_with_leading_zeros =
[bfe_vec!(coefficient_encoding.len()), coefficient_encoding].concat();
let decoding_result = Polynomial::<FF>::decode(&poly_encoding_with_leading_zeros);
prop_assert!(matches!(
decoding_result.unwrap_err(),
PolynomialBFieldCodecError::TrailingZerosInPolynomialEncoding
Expand All @@ -799,7 +789,7 @@ mod tests {

#[proptest]
fn disallow_trailing_zeros_in_poly_encoding_bfe(
polynomial: Polynomial<BFieldElement>,
polynomial: Polynomial<'static, BFieldElement>,
#[filter(!#leading_coefficient.is_zero())] leading_coefficient: BFieldElement,
#[strategy(1usize..30)] num_leading_zeros: usize,
) {
Expand All @@ -812,7 +802,7 @@ mod tests {

#[proptest]
fn disallow_trailing_zeros_in_poly_encoding_xfe(
polynomial: Polynomial<XFieldElement>,
polynomial: Polynomial<'static, XFieldElement>,
#[filter(!#leading_coefficient.is_zero())] leading_coefficient: XFieldElement,
#[strategy(1usize..30)] num_leading_zeros: usize,
) {
Expand Down Expand Up @@ -891,20 +881,20 @@ mod tests {

// Structs containing Polynomial<T> where T is BFieldCodec
#[derive(Debug, Clone, PartialEq, Eq, BFieldCodec, Arbitrary)]
struct DeriveTestStructWithBfePolynomialField(Polynomial<BFieldElement>);
struct DeriveTestStructWithBfePolynomialField(Polynomial<'static, BFieldElement>);

test_case! { fn struct_with_bfe_poly_field for DeriveTestStructWithBfePolynomialField: None }

#[derive(Debug, Clone, PartialEq, Eq, BFieldCodec, Arbitrary)]
struct DeriveTestStructWithXfePolynomialField(Polynomial<XFieldElement>);
struct DeriveTestStructWithXfePolynomialField(Polynomial<'static, XFieldElement>);

test_case! { fn struct_with_xfe_poly_field for DeriveTestStructWithXfePolynomialField: None }

#[derive(Debug, Clone, PartialEq, Eq, BFieldCodec, Arbitrary)]
enum EnumWithVariantWithPolyField {
Bee,
BooBoo(Polynomial<BFieldElement>),
Bop(Polynomial<XFieldElement>),
BooBoo(Polynomial<'static, BFieldElement>),
Bop(Polynomial<'static, XFieldElement>),
}

test_case! { fn enum_with_variant_with_poly_field for EnumWithVariantWithPolyField: None }
Expand Down Expand Up @@ -1283,32 +1273,35 @@ mod tests {
coefficients: Vec<T>,
}

impl<T: FiniteField + BFieldCodec> From<Polynomial<T>> for DummyPolynomial<T> {
fn from(value: Polynomial<T>) -> Self {
Self {
coefficients: value.coefficients,
}
}
}

#[proptest]
fn manual_poly_encoding_implementation_is_consistent_with_derived_bfe(
mut polynomial: Polynomial<BFieldElement>,
#[filter(!#leading_coefficient.is_zero())] leading_coefficient: BFieldElement,
#[strategy(arb())] coefficients: Vec<BFieldElement>,
) {
polynomial.coefficients.push(leading_coefficient);
let as_dummy: DummyPolynomial<BFieldElement> = polynomial.clone().into();
prop_assert_eq!(as_dummy.encode(), polynomial.encode());
manual_poly_encoding_implementation_is_consistent_with_derived(coefficients)?;
}

#[proptest]
fn manual_poly_encoding_implementation_is_consistent_with_derived_xfe(
mut polynomial: Polynomial<XFieldElement>,
#[filter(!#leading_coefficient.is_zero())] leading_coefficient: XFieldElement,
#[strategy(arb())] coefficients: Vec<XFieldElement>,
) {
polynomial.coefficients.push(leading_coefficient);
let as_dummy: DummyPolynomial<XFieldElement> = polynomial.clone().into();
prop_assert_eq!(as_dummy.encode(), polynomial.encode());
manual_poly_encoding_implementation_is_consistent_with_derived(coefficients)?;
}

fn manual_poly_encoding_implementation_is_consistent_with_derived<FF>(
coefficients: Vec<FF>,
) -> TestCaseResult
where
FF: FiniteField + BFieldCodec + 'static,
{
if coefficients.last().is_some_and(Zero::is_zero) {
let rsn = "`DummyPolynomial::encode` only works for non-zero leading coefficients";
return Err(TestCaseError::Reject(rsn.into()));
}

let polynomial = Polynomial::new(coefficients.clone());
let dummy_polynomial = DummyPolynomial { coefficients };
prop_assert_eq!(dummy_polynomial.encode(), polynomial.encode());
Ok(())
}
}
}
Loading