batch proving with same number of variables - part 1 #397
Conversation
tcoratger
left a comment
tcoratger
left a comment
There was a problem hiding this comment.
Just left a couple of comments. I'm not sure about the usefulness of the added tests.
They don't seem to directly test the functions in the file, but rather similar behaviors, which is pretty weird because then we don't have any complete tests calling batch_prove or selector_round functions.
Co-authored-by: Thomas Coratger <[email protected]>
Co-authored-by: Thomas Coratger <[email protected]>
Co-authored-by: Thomas Coratger <[email protected]>
Co-authored-by: Thomas Coratger <[email protected]>
Co-authored-by: Thomas Coratger <[email protected]>
Co-authored-by: Thomas Coratger <[email protected]>
Co-authored-by: Thomas Coratger <[email protected]>
…e same OOD points since we are sampling after the commitment of them
|
You are right the tests were ill-formed. I added a new test for |
| let alpha: EF = challenger.sample_algebra_element(); | ||
|
|
||
| // ==== Step 3: Extract claims from statements ==== | ||
| // For now, we assume single-constraint statements |
There was a problem hiding this comment.
No of course I have no problem to merge it as fast as possible. Even on the contrary, I prefer that we merge a first iteration with polynomials of same size and single constraint so that we can have a first idea and then iterate on the rest with followup PRs.
But I would still like us to have an end to end test with single constraint and for example 2 polynomials of the same size where the prove/verify workflow is working so that we are sure what we are doing is fully right.
Does this make sense?
My goal here is to make sure we don't merge code that is false and not working. In our case I think that the only way to confirm everything is working fine is by running the full end to end test.
Co-authored-by: Thomas Coratger <[email protected]>
|
Should we move first ood points sampling from CommitmentWriter::commit to Prover::prove. So that this PR can also use CommitmentWriter::commit |
| // Compute sumcheck polynomial coefficients: | ||
| // h(X) = Σ_b f_c(X, b) · w(X, b) | ||
| // We compute c0 = h(0) and c2 (quadratic coefficient) | ||
| let (c0, c2) = combined_poly.sumcheck_coefficients(&combined_weights); |
There was a problem hiding this comment.
I'm not sure if we should allocate two new large space for concatenated polys. Notice that in sumcheck polys we already split them in the middle. Maybe for this use case we can introduce a new sumcheck_coefficient function instead of more memory usage
| // σ' = h(r_0) | ||
|
|
||
| // Folded polynomial in extension field | ||
| let g = EvaluationsList::linear_combination(f_a, r_0, f_b, EF::ONE - r_0); |
There was a problem hiding this comment.
Similarly maybe it is possible to combine f_a and f_b onto f_a by getting f_a with mut value
| .iter() | ||
| .zip(eq_z_b.iter()) | ||
| .map(|(&a, &b)| r_0 * a + (EF::ONE - r_0) * b) | ||
| .collect(); |
There was a problem hiding this comment.
- We can do it with only one multiplication r*(a-b) + b
- Or constructing eqs with r like
EL::new_from_point(z_a.as_slice(), alpha *r )andEL::new_from_point(z_a.as_slice(), alpha *(r-1))and simple sum would work? - Maybe this sum can be better in parallel
3 participants
Addressing issue:
#228