o1-labs / proof-systems

The proof systems used by Mina
https://o1-labs.github.io/proof-systems/
Apache License 2.0
410 stars 91 forks source link

ProofEvaluations is too generic #231

Closed mimoo closed 1 year ago

mimoo commented 2 years ago
pub struct ProofEvaluations<Field> {
    /// witness polynomials
    pub w: [Field; COLUMNS],
    /// permutation polynomial
    pub z: Field,
    /// permutation polynomials
    /// (PERMUTS-1 evaluations because the last permutation is only used in commitment form)
    pub s: [Field; PERMUTS - 1],
    /// lookup-related evaluations
    pub lookup: Option<LookupEvaluations<Field>>,
    /// evaluation of the generic selector polynomial
    pub generic_selector: Field,
    /// evaluation of the poseidon selector polynomial
    pub poseidon_selector: Field,
}

should be

pub struct ProofEvaluations<Field> {
    /// witness polynomials
    pub w: [Vec<Field>; COLUMNS],
    /// permutation polynomial
    pub z: Vec<Field>,
    /// permutation polynomials
    /// (PERMUTS-1 evaluations because the last permutation is only used in commitment form)
    pub s: [Vec<Field>; PERMUTS - 1],
    /// lookup-related evaluations
    pub lookup: Option<LookupEvaluations<Vec<Field>>>,
    /// evaluation of the generic selector polynomial
    pub generic_selector: Vec<Field>,
    /// evaluation of the poseidon selector polynomial
    pub poseidon_selector: Vec<Field>,
}

it'd be nice if we actually abstracted chunked evaluations, chunked polynomials, chunked commitments, etc.

github-actions[bot] commented 2 years ago

Stale issue message

mimoo commented 2 years ago

we might not be able to tackle this one without https://github.com/o1-labs/proof-systems/issues/432 first

github-actions[bot] commented 2 years ago

Stale issue message

mimoo commented 2 years ago

I think we might be able to just write this struct as:

pub struct ProofEvaluations<F> where F: FftField {
    /// witness polynomials
    pub w: [F; COLUMNS],
    /// permutation polynomial
    pub z: F,
    /// permutation polynomials
    /// (PERMUTS-1 evaluations because the last permutation is only used in commitment form)
    pub s: [F; PERMUTS - 1],
    /// lookup-related evaluations
    pub lookup: Option<LookupEvaluations<F>>,
    /// evaluation of the generic selector polynomial
    pub generic_selector: F,
    /// evaluation of the poseidon selector polynomial
    pub poseidon_selector: F,
}

as none of these evaluations are chunked (unless I'm misunderstanding something)

querolita commented 2 years ago

Let's try something like:

pub struct ChunkedEvaluations<F> (ProofEvaluations<Vec<F>>);

instead of duplicating manually all chunked types, and still keep ProofEvaluations generic enough

mimoo commented 2 years ago

I tried removing the Vec and just making them single elements: https://github.com/o1-labs/proof-systems/pull/680

tests don't pass due to how the PCS is written, specifically when something evaluates to zero it expects you to pass an empty vec: https://github.com/o1-labs/proof-systems/blob/master/poly-commitment/src/commitment.rs#L434

    for (evals_tr, shifted) in polys.iter().filter(|(evals_tr, _)| !evals_tr[0].is_empty()) {
querolita commented 2 years ago

Closing in sight of issue #680

mimoo commented 2 years ago

if you don't mind let's reopen it until I test this on the mina side, as I predict that it might be a bit painful

github-actions[bot] commented 2 years ago

Stale issue message

github-actions[bot] commented 1 year ago

Stale issue message

mimoo commented 1 year ago

truer today than ever :D I think we will need to look into simplifying that part at some point. The problem now is all the generics:

pub struct PointEvaluations<Evals> {
    /// Evaluation at the challenge point zeta.
    pub zeta: Evals,
    /// Evaluation at `zeta . omega`, the product of the challenge point and the group generator.
    pub zeta_omega: Evals,
}

pub struct ProofEvaluations<Evals> {
    /// witness polynomials
    pub w: [Evals; COLUMNS],
    /// permutation polynomial
    pub z: Evals,
    /// permutation polynomials
    /// (PERMUTS-1 evaluations because the last permutation is only used in commitment form)
    pub s: [Evals; PERMUTS - 1],
    /// coefficient polynomials
    pub coefficients: [Evals; COLUMNS],
    /// lookup-related evaluations
    pub lookup: Option<LookupEvaluations<Evals>>,
    /// evaluation of the generic selector polynomial
    pub generic_selector: Evals,
    /// evaluation of the poseidon selector polynomial
    pub poseidon_selector: Evals,
}

which we instantiate like that:

pub evals: ProofEvaluations<PointEvaluations<Vec<G::ScalarField>>>,
github-actions[bot] commented 1 year ago

Stale issue message

github-actions[bot] commented 1 year ago

Stale issue message