Use only canonic domains

[spapinistarkware]

This change is 

[graphite-app]

Your org has enabled the Graphite merge queue for merging into dev

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

[spapinistarkware]

• parallel wide fib test #511
• Evaluation oods once #496
• Reuse commitment evaluation #495
• Use precomputed twiddles #494
• Add measurement logs #493
• WideFib test with AVX Backend #492
• Implement Backend for AVX512Backend #491
• Make prove() generic in Backend #490
• CommitmentSchemeProver generic in backend #489
• AVX quotients #488
• Clean up fri quotients #486
• Save inverse on quotient numerator #484
• Commitment Scheme evaluation per size #483
• Separate commitment prover and verifier to modules #482
• QuotientOps #481
• DomainAccumulator allows accumulating a row at once #480
• Rename evalaution.rs to accumulaion.rs #479
• Move SecureColumn #478
• Use only canonic domains #463 👈
• Revert "integrated batch inverse in fib cpu (#448)" #477
• FRI in Commitment Scheme #476
• Field debug implementations #501
• CommitmentScheme utils #500
• Air with dyn component #487
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @spapinistarkware and the rest of your teammates on Graphite

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.43%. Comparing base (6443804) to head (4dc88e9).

Additional details and impacted files

@@              Coverage Diff              @@
##           Revert448     #463      +/-   ##
=============================================
- Coverage      95.47%   95.43%   -0.04%     
=============================================
  Files             59       59              
  Lines           8795     8699      -96     
  Branches        8795     8699      -96     
=============================================
- Hits            8397     8302      -95     
+ Misses           350      347       -3     
- Partials          48       50       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[shaharsamocha7]

Reviewed 3 of 10 files at r1, 3 of 7 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: 9 of 10 files reviewed, 6 unresolved discussions (waiting on @spapinistarkware)

---

src/core/air/evaluation.rs line 172 at r3 (raw file):

                cols: values.cols.map(|c| {
                    CPUCircleEvaluation::<_, BitReversedOrder>::new(
                        CanonicCoset::new(log_size as u32).circle_domain(),

Suggestion:

CanonicCoset::new(log_size).circle_domain()

---

src/core/air/evaluation.rs line 307 at r3 (raw file):

        for (log_size, values) in pairs.into_iter() {
            res = res * alpha
                + CPUCircleEvaluation::new(CanonicCoset::new(log_size).circle_domain(), values)

IMO keep the constraint_evaluation_domain function
and put this code inside

Code quote:

CanonicCoset::new(log_size).circle_domain()

---

src/core/poly/circle/domain.rs line 110 at r3 (raw file):

    #[test]
    fn test_circle_domain_iterator() {

I think that you should change this test to work with CanonicCoset circle domain.
Do we have another test for the iterator?

Code quote:

fn test_circle_domain_iterator() {

---

src/fibonacci/component.rs line 108 at r3 (raw file):

                let res = self.step_constraint_eval_quotient_by_mask(point, &mask);
                accum.accumulate(bit_reverse_index(i + off, constraint_log_degree_bound), res);
                let res = self.boundary_constraint_eval_quotient_by_mask(point, &[mask[0]]);

I think that res should be renamed

Code quote:

                let res = self.step_constraint_eval_quotient_by_mask(point, &mask);
                accum.accumulate(bit_reverse_index(i + off, constraint_log_degree_bound), res);
                let res = self.boundary_constraint_eval_quotient_by_mask(point, &[mask[0]]);

---

src/fibonacci/component.rs line 113 at r3 (raw file):

        // Boundary constraint.
        let constraint_log_degree_bound = trace_domain.log_size();

Why did this code go?

Code quote:

        // Boundary constraint.
        let constraint_log_degree_bound = trace_domain.log_size();

---

src/fibonacci/component.rs line 130 at r3 (raw file):

        let res = self
            .boundary_constraint_eval_quotient_by_mask(point, &mask[0][..1].try_into().unwrap());
        let constraint_log_degree_bound = self.log_size + 1;

huh?

Code quote:

self.log_size + 1

[spapinistarkware]

Reviewable status: 9 of 10 files reviewed, 6 unresolved discussions (waiting on @shaharsamocha7)

---

src/core/air/evaluation.rs line 172 at r3 (raw file):

                cols: values.cols.map(|c| {
                    CPUCircleEvaluation::<_, BitReversedOrder>::new(
                        CanonicCoset::new(log_size as u32).circle_domain(),

It's usize

---

src/core/air/evaluation.rs line 307 at r3 (raw file):

Previously, shaharsamocha7 wrote…

IMO keep the constraint_evaluation_domain function
and put this code inside

It's not the constraint evalaution domain no more. What name should I give it then?

---

src/core/poly/circle/domain.rs line 110 at r3 (raw file):

Previously, shaharsamocha7 wrote…

I think that you should change this test to work with CanonicCoset circle domain.
Do we have another test for the iterator?

Done.

---

src/fibonacci/component.rs line 108 at r3 (raw file):

Previously, shaharsamocha7 wrote…

I think that res should be renamed

Done.

---

src/fibonacci/component.rs line 113 at r3 (raw file):

Previously, shaharsamocha7 wrote…

Why did this code go?

The 2 constraints are now computed on the same domain

---

src/fibonacci/component.rs line 130 at r3 (raw file):

Previously, shaharsamocha7 wrote…

huh?

Now both constraints are computed on the same domain. These sizes should match the sizes in eval_on_domain

[shaharsamocha7]

Reviewed 4 of 4 files at r4, all commit messages.
Reviewable status: 9 of 10 files reviewed, 3 unresolved discussions (waiting on @spapinistarkware)

---

src/core/poly/circle/domain.rs line 110 at r3 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

Done.

Using a CanonicCoset?
Actually CanonicCoset iteration can be done in two ways (right?), so maybe the test can be better.

---

src/fibonacci/component.rs line 128 at r4 (raw file):

        evaluation_accumulator: &mut PointEvaluationAccumulator,
    ) {
        let constraint_log_degree_bound = self.log_size + 1;

Suggestion:

let constraints_log_degree_bound = self.log_size + 1;

---

src/fibonacci/component.rs line 133 at r4 (raw file):

            self.step_constraint_eval_quotient_by_mask(point, &mask[0][..].try_into().unwrap()),
        );
        let constraint_log_degree_bound = self.log_size + 1;

Remove

Code quote:

let constraint_log_degree_bound = self.log_size + 1;

[spapinistarkware]

Reviewable status: 6 of 10 files reviewed, 3 unresolved discussions (waiting on @shaharsamocha7)

---

src/fibonacci/component.rs line 128 at r4 (raw file):

        evaluation_accumulator: &mut PointEvaluationAccumulator,
    ) {
        let constraint_log_degree_bound = self.log_size + 1;

Done.

---

src/fibonacci/component.rs line 133 at r4 (raw file):

Previously, shaharsamocha7 wrote…

Remove

Done.

[shaharsamocha7]

Reviewed 4 of 4 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @spapinistarkware)

[spapinistarkware]

Merge activity

• Mar 21, 7:23 AM EDT: @spapinistarkware started a stack merge that includes this pull request via Graphite.
• Mar 21, 7:24 AM EDT: @spapinistarkware merged this pull request with Graphite.