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.

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

chore: make FpOperations use PrimeField32 to clean up type inconsistency hacks #203

Merged
merged 11 commits into from
Feb 13, 2024
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::params::Limbs;
use super::params::NUM_WITNESS_LIMBS;
use super::params::{convert_polynomial, convert_vec, Limbs};
use super::util::{compute_root_quotient_and_shift, split_u16_limbs_to_u8_limbs};
use super::util_air::eval_field_operation;
use crate::air::CurtaAirBuilder;
Expand All @@ -8,38 +8,34 @@ use crate::utils::ec::field::FieldParameters;
use core::borrow::{Borrow, BorrowMut};
use core::mem::size_of;
use num::BigUint;
use p3_baby_bear::BabyBear;
use p3_field::Field;
use p3_field::PrimeField32;
use std::fmt::Debug;
use valida_derive::AlignedBorrow;

// a / (1 + b) if sign
// a/ -b if !sign
/// A set of columns to compute `FpDen(a, b)` where a, b are field elements.
/// A set of columns to compute `FieldDen(a, b)` where `a`, `b` are field elements.
///
/// `a / (1 + b)` if `sign`
/// `a / -b` if `!sign`
///
/// Right now the number of limbs is assumed to be a constant, although this could be macro-ed
/// or made generic in the future.
#[derive(Debug, Clone, AlignedBorrow)]
#[repr(C)]
pub struct FpDenCols<T> {
pub struct FieldDenCols<T> {
/// The result of `a den b`, where a, b are field elements
pub result: Limbs<T>,
pub(crate) carry: Limbs<T>,
pub(crate) witness_low: [T; NUM_WITNESS_LIMBS],
pub(crate) witness_high: [T; NUM_WITNESS_LIMBS],
}

impl<F: Field> FpDenCols<F> {
impl<F: PrimeField32> FieldDenCols<F> {
pub fn populate<P: FieldParameters>(
&mut self,
a: &BigUint,
b: &BigUint,
sign: bool,
) -> BigUint {
/// TODO: This operation relies on `F` being a PrimeField32, but our traits do not
/// support that. This is a hack, since we always use BabyBear, to get around that, but
/// all operations using "PF" should use "F" in the future.
type PF = BabyBear;

let p = P::modulus();
let minus_b_int = &p - b;
let b_signed = if sign { b.clone() } else { minus_b_int };
Expand All @@ -59,11 +55,11 @@ impl<F: Field> FpDenCols<F> {
debug_assert!(carry < p);
debug_assert_eq!(&carry * &p, &equation_lhs - &equation_rhs);

let p_a: Polynomial<PF> = P::to_limbs_field::<PF>(a).into();
let p_b: Polynomial<PF> = P::to_limbs_field::<PF>(b).into();
let p_p: Polynomial<PF> = P::to_limbs_field::<PF>(&p).into();
let p_result: Polynomial<PF> = P::to_limbs_field::<PF>(&result).into();
let p_carry: Polynomial<PF> = P::to_limbs_field::<PF>(&carry).into();
let p_a: Polynomial<F> = P::to_limbs_field::<F>(a).into();
let p_b: Polynomial<F> = P::to_limbs_field::<F>(b).into();
let p_p: Polynomial<F> = P::to_limbs_field::<F>(&p).into();
let p_result: Polynomial<F> = P::to_limbs_field::<F>(&result).into();
let p_carry: Polynomial<F> = P::to_limbs_field::<F>(&carry).into();

// Compute the vanishing polynomial.
let vanishing_poly = if sign {
Expand All @@ -80,16 +76,16 @@ impl<F: Field> FpDenCols<F> {
);
let (p_witness_low, p_witness_high) = split_u16_limbs_to_u8_limbs(&p_witness);

self.result = convert_polynomial(p_result);
self.carry = convert_polynomial(p_carry);
self.witness_low = convert_vec(p_witness_low).try_into().unwrap();
self.witness_high = convert_vec(p_witness_high).try_into().unwrap();
self.result = p_result.into();
self.carry = p_carry.into();
self.witness_low = p_witness_low.try_into().unwrap();
self.witness_high = p_witness_high.try_into().unwrap();

result
}
}

impl<V: Copy> FpDenCols<V> {
impl<V: Copy> FieldDenCols<V> {
#[allow(unused_variables)]
pub fn eval<AB: CurtaAirBuilder<Var = V>, P: FieldParameters>(
&self,
Expand Down Expand Up @@ -132,9 +128,9 @@ impl<V: Copy> FpDenCols<V> {
mod tests {
use num::BigUint;
use p3_air::BaseAir;
use p3_field::Field;
use p3_field::{Field, PrimeField32};

use super::{FpDenCols, Limbs};
use super::{FieldDenCols, Limbs};
use crate::air::MachineAir;
use crate::utils::ec::edwards::ed25519::Ed25519BaseField;
use crate::utils::ec::field::FieldParameters;
Expand All @@ -154,17 +150,17 @@ mod tests {
pub struct TestCols<T> {
pub a: Limbs<T>,
pub b: Limbs<T>,
pub a_den_b: FpDenCols<T>,
pub a_den_b: FieldDenCols<T>,
}

pub const NUM_TEST_COLS: usize = size_of::<TestCols<u8>>();

struct FpDenChip<P: FieldParameters> {
struct FieldDenChip<P: FieldParameters> {
pub sign: bool,
pub _phantom: std::marker::PhantomData<P>,
}

impl<P: FieldParameters> FpDenChip<P> {
impl<P: FieldParameters> FieldDenChip<P> {
pub fn new(sign: bool) -> Self {
Self {
sign,
Expand All @@ -173,9 +169,9 @@ mod tests {
}
}

impl<F: Field, P: FieldParameters> MachineAir<F> for FpDenChip<P> {
impl<F: PrimeField32, P: FieldParameters> MachineAir<F> for FieldDenChip<P> {
fn name(&self) -> String {
"FpDen".to_string()
"FieldDen".to_string()
}

fn shard(&self, _: &ExecutionRecord, _: &mut Vec<ExecutionRecord>) {}
Expand Down Expand Up @@ -227,13 +223,13 @@ mod tests {
}
}

impl<F: Field, P: FieldParameters> BaseAir<F> for FpDenChip<P> {
impl<F: Field, P: FieldParameters> BaseAir<F> for FieldDenChip<P> {
fn width(&self) -> usize {
NUM_TEST_COLS
}
}

impl<AB, P: FieldParameters> Air<AB> for FpDenChip<P>
impl<AB, P: FieldParameters> Air<AB> for FieldDenChip<P>
where
AB: CurtaAirBuilder,
{
Expand All @@ -254,7 +250,7 @@ mod tests {
#[test]
fn generate_trace() {
let mut shard = ExecutionRecord::default();
let chip: FpDenChip<Ed25519BaseField> = FpDenChip::new(true);
let chip: FieldDenChip<Ed25519BaseField> = FieldDenChip::new(true);
let trace: RowMajorMatrix<BabyBear> = chip.generate_trace(&mut shard);
println!("{:?}", trace.values)
}
Expand All @@ -266,7 +262,7 @@ mod tests {

let mut shard = ExecutionRecord::default();

let chip: FpDenChip<Ed25519BaseField> = FpDenChip::new(true);
let chip: FieldDenChip<Ed25519BaseField> = FieldDenChip::new(true);
let trace: RowMajorMatrix<BabyBear> = chip.generate_trace(&mut shard);
// This it to test that the proof DOESN'T work if messed up.
// let row = trace.row_mut(0);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::params::Limbs;
use super::params::NUM_WITNESS_LIMBS;
use super::params::{convert_polynomial, convert_vec, Limbs};
use super::util::{compute_root_quotient_and_shift, split_u16_limbs_to_u8_limbs};
use super::util_air::eval_field_operation;
use crate::air::CurtaAirBuilder;
Expand All @@ -9,40 +9,29 @@ use core::borrow::{Borrow, BorrowMut};
use core::mem::size_of;
use num::BigUint;
use num::Zero;
use p3_baby_bear::BabyBear;
use p3_field::AbstractField;
use p3_field::Field;
use p3_field::{AbstractField, PrimeField32};
use std::fmt::Debug;
use valida_derive::AlignedBorrow;

/// A set of columns to compute `FpInnerProduct(Vec<a>, Vec<b>)` where a, b are field elements.
/// A set of columns to compute `FieldInnerProduct(Vec<a>, Vec<b>)` where a, b are field elements.
/// Right now the number of limbs is assumed to be a constant, although this could be macro-ed
/// or made generic in the future.
#[derive(Debug, Clone, AlignedBorrow)]
#[repr(C)]
pub struct FpInnerProductCols<T> {
pub struct FieldInnerProductCols<T> {
/// The result of `a inner product b`, where a, b are field elements
pub result: Limbs<T>,
pub(crate) carry: Limbs<T>,
pub(crate) witness_low: [T; NUM_WITNESS_LIMBS],
pub(crate) witness_high: [T; NUM_WITNESS_LIMBS],
}

impl<F: Field> FpInnerProductCols<F> {
impl<F: PrimeField32> FieldInnerProductCols<F> {
pub fn populate<P: FieldParameters>(&mut self, a: &[BigUint], b: &[BigUint]) -> BigUint {
/// TODO: This operation relies on `F` being a PrimeField32, but our traits do not
/// support that. This is a hack, since we always use BabyBear, to get around that, but
/// all operations using "PF" should use "F" in the future.
type PF = BabyBear;

let p_a_vec: Vec<Polynomial<PF>> = a
.iter()
.map(|x| P::to_limbs_field::<PF>(x).into())
.collect();
let p_b_vec: Vec<Polynomial<PF>> = b
.iter()
.map(|x| P::to_limbs_field::<PF>(x).into())
.collect();
let p_a_vec: Vec<Polynomial<F>> =
a.iter().map(|x| P::to_limbs_field::<F>(x).into()).collect();
let p_b_vec: Vec<Polynomial<F>> =
b.iter().map(|x| P::to_limbs_field::<F>(x).into()).collect();

let modulus = &P::modulus();
let inner_product = a
Expand All @@ -56,15 +45,15 @@ impl<F: Field> FpInnerProductCols<F> {
assert!(carry < &(2u32 * modulus));
assert_eq!(carry * modulus, inner_product - result);

let p_modulus: Polynomial<PF> = P::to_limbs_field::<PF>(modulus).into();
let p_result: Polynomial<PF> = P::to_limbs_field::<PF>(result).into();
let p_carry: Polynomial<PF> = P::to_limbs_field::<PF>(carry).into();
let p_modulus: Polynomial<F> = P::to_limbs_field::<F>(modulus).into();
let p_result: Polynomial<F> = P::to_limbs_field::<F>(result).into();
let p_carry: Polynomial<F> = P::to_limbs_field::<F>(carry).into();

// Compute the vanishing polynomial.
let p_inner_product = p_a_vec
.into_iter()
.zip(p_b_vec)
.fold(Polynomial::<PF>::new(vec![PF::zero()]), |acc, (c, d)| {
.fold(Polynomial::<F>::new(vec![F::zero()]), |acc, (c, d)| {
acc + &c * &d
});
let p_vanishing = p_inner_product - &p_result - &p_carry * &p_modulus;
Expand All @@ -77,16 +66,16 @@ impl<F: Field> FpInnerProductCols<F> {
);
let (p_witness_low, p_witness_high) = split_u16_limbs_to_u8_limbs(&p_witness);

self.result = convert_polynomial(p_result);
self.carry = convert_polynomial(p_carry);
self.witness_low = convert_vec(p_witness_low).try_into().unwrap();
self.witness_high = convert_vec(p_witness_high).try_into().unwrap();
self.result = p_result.into();
self.carry = p_carry.into();
self.witness_low = p_witness_low.try_into().unwrap();
self.witness_high = p_witness_high.try_into().unwrap();

result.clone()
}
}

impl<V: Copy> FpInnerProductCols<V> {
impl<V: Copy> FieldInnerProductCols<V> {
#[allow(unused_variables)]
pub fn eval<AB: CurtaAirBuilder<Var = V>, P: FieldParameters>(
&self,
Expand Down Expand Up @@ -127,9 +116,9 @@ impl<V: Copy> FpInnerProductCols<V> {
mod tests {
use num::BigUint;
use p3_air::BaseAir;
use p3_field::Field;
use p3_field::{Field, PrimeField32};

use super::{FpInnerProductCols, Limbs};
use super::{FieldInnerProductCols, Limbs};
use crate::air::MachineAir;
use crate::utils::ec::edwards::ed25519::Ed25519BaseField;
use crate::utils::ec::field::FieldParameters;
Expand All @@ -150,26 +139,26 @@ mod tests {
pub struct TestCols<T> {
pub a: [Limbs<T>; 1],
pub b: [Limbs<T>; 1],
pub a_ip_b: FpInnerProductCols<T>,
pub a_ip_b: FieldInnerProductCols<T>,
}

pub const NUM_TEST_COLS: usize = size_of::<TestCols<u8>>();

struct FpIpChip<P: FieldParameters> {
struct FieldIpChip<P: FieldParameters> {
pub _phantom: std::marker::PhantomData<P>,
}

impl<P: FieldParameters> FpIpChip<P> {
impl<P: FieldParameters> FieldIpChip<P> {
pub fn new() -> Self {
Self {
_phantom: std::marker::PhantomData,
}
}
}

impl<F: Field, P: FieldParameters> MachineAir<F> for FpIpChip<P> {
impl<F: PrimeField32, P: FieldParameters> MachineAir<F> for FieldIpChip<P> {
fn name(&self) -> String {
"FpInnerProduct".to_string()
"FieldInnerProduct".to_string()
}

fn shard(&self, _: &ExecutionRecord, _: &mut Vec<ExecutionRecord>) {}
Expand Down Expand Up @@ -219,13 +208,13 @@ mod tests {
}
}

impl<F: Field, P: FieldParameters> BaseAir<F> for FpIpChip<P> {
impl<F: Field, P: FieldParameters> BaseAir<F> for FieldIpChip<P> {
fn width(&self) -> usize {
NUM_TEST_COLS
}
}

impl<AB, P: FieldParameters> Air<AB> for FpIpChip<P>
impl<AB, P: FieldParameters> Air<AB> for FieldIpChip<P>
where
AB: CurtaAirBuilder,
{
Expand All @@ -245,7 +234,7 @@ mod tests {
#[test]
fn generate_trace() {
let mut shard = ExecutionRecord::default();
let chip: FpIpChip<Ed25519BaseField> = FpIpChip::new();
let chip: FieldIpChip<Ed25519BaseField> = FieldIpChip::new();
let trace: RowMajorMatrix<BabyBear> = chip.generate_trace(&mut shard);
println!("{:?}", trace.values)
}
Expand All @@ -257,7 +246,7 @@ mod tests {

let mut shard = ExecutionRecord::default();

let chip: FpIpChip<Ed25519BaseField> = FpIpChip::new();
let chip: FieldIpChip<Ed25519BaseField> = FieldIpChip::new();
let trace: RowMajorMatrix<BabyBear> = chip.generate_trace(&mut shard);
let proof = prove::<BabyBearPoseidon2, _>(&config, &chip, &mut challenger, trace);

Expand Down
Loading
Loading