Add wide fib lookup step constraint.

[alonh5]

This change is 

[alonh5]

• Create phase number constants. #669 : 3 dependent PRs (#671 , #677 , #679 )
• Add final lookup constraint. #667
• Add lookup final boundary constraints. #664
• Separate constraint point evaluation to functions. #662
• Add wide fib lookup step constraint. #646 👈
• Extract constraint code into functions. #644 : 1 other dependent PR (#654 )
• Use SecureField in interaction. #642
• Modify mask_points for interaction. #639
• Modify trace_log_degree_bounds for interaction. #638
• Add wide fib lookup constraint. #637
• Add interaction to proof. #636
• Separate componenet and air trace writers. #651
• dev

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

Join @alonh5 and the rest of your teammates on Graphite

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.03%. Comparing base (a759ce0) to head (ae26ec4).
Report is 2 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #646      +/-   ##
==========================================
+ Coverage   89.95%   90.03%   +0.07%     
==========================================
  Files          75       75              
  Lines        9657     9730      +73     
  Branches     9657     9730      +73     
==========================================
+ Hits         8687     8760      +73     
  Misses        889      889              
  Partials       81       81              

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

[alonh5]

Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7 and @spapinistarkware)

---

crates/prover/src/core/utils.rs line 76 at r4 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

1. Do we need these in the long term? If not, add a TODO to remove. If yes, they are currently not efficient, and seem to only do things in the CPU, and not in a backend. Also, should the oeprate on Vec, and no on Evaluation or something?

Done.

[shaharsamocha7]

Reviewed 3 of 7 files at r2, 2 of 3 files at r5, 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @alonh5 and @spapinistarkware)

---

crates/prover/src/core/utils.rs line 71 at r6 (raw file):

        prev_index = (prev_index + half_size - 1) % half_size;
    } else {
        prev_index = ((prev_index + 1) % half_size) + half_size;

Does it work for constraint degree > 2?
I think that you need the extension degree as well

Suggestion:

        prev_index = ((prev_index + extension_degree / 2) % half_size) + half_size;

---

crates/prover/src/examples/wide_fibonacci/component.rs line 159 at r6 (raw file):

            .map(|eval| {
                let coset = CanonicCoset::new(trace[0].domain.log_size());
                CpuCircleEvaluation::new_canonical_ordered(coset, eval)

What is this change? Adding bit reverse?

Code quote:

                let coset = CanonicCoset::new(trace[0].domain.log_size());
                CpuCircleEvaluation::new_canonical_ordered(coset, eval)

---

crates/prover/src/examples/wide_fibonacci/constraint_eval.rs line 137 at r6 (raw file):

    }

    fn evaluate_lookup_step_constraints(

Let's add a TODO to try to simplify this function (moving parts to utility functions)
not for this pr

Code quote:

    fn evaluate_lookup_step_constraints(

---

crates/prover/src/examples/wide_fibonacci/mod.rs line 153 at r6 (raw file):

            .into_iter()
            .map(|eval| CpuCircleEvaluation::new_canonical_ordered(trace_domain, eval))
            .collect_vec();

This applies bit reverse on the entire trace.
It would be better to just bit reverse the lookup column?

Code quote:

        let trace = trace_values
            .into_iter()
            .map(|eval| CpuCircleEvaluation::new_canonical_ordered(trace_domain, eval))
            .collect_vec();

---

crates/prover/src/examples/wide_fibonacci/trace_gen.rs line 50 at r6 (raw file):

            circle_domain_order_to_coset_order(column)
        })
        .collect_vec();

Add a TODO, the better thing to do is to bit reverse only the expressions that relevant for the lookup
instead of bit reversing the entire trace which can be expensive

Code quote:

    let natural_ordered_trace = input_trace
        .iter_mut()
        .map(|column| {
            bit_reverse(column);
            circle_domain_order_to_coset_order(column)
        })
        .collect_vec();

[alonh5]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @shaharsamocha7 and @spapinistarkware)

---

crates/prover/src/core/utils.rs line 71 at r6 (raw file):

Previously, shaharsamocha7 wrote…

Does it work for constraint degree > 2?
I think that you need the extension degree as well

Yes I wrote it in the documentation, but i generalized it now.

---

crates/prover/src/examples/wide_fibonacci/component.rs line 159 at r6 (raw file):

Previously, shaharsamocha7 wrote…

What is this change? Adding bit reverse?

It reorders it in circle domain order and also bit reverses. This is needed for constraints between rows to be consistent.

---

crates/prover/src/examples/wide_fibonacci/constraint_eval.rs line 137 at r6 (raw file):

Previously, shaharsamocha7 wrote…

Let's add a TODO to try to simplify this function (moving parts to utility functions)
not for this pr

Done.

---

crates/prover/src/examples/wide_fibonacci/mod.rs line 153 at r6 (raw file):

Previously, shaharsamocha7 wrote…

This applies bit reverse on the entire trace.
It would be better to just bit reverse the lookup column?

I need to turn the trace order into circle domain and bit reverse for the same reason above.

---

crates/prover/src/examples/wide_fibonacci/trace_gen.rs line 50 at r6 (raw file):

Previously, shaharsamocha7 wrote…

Add a TODO, the better thing to do is to bit reverse only the expressions that relevant for the lookup
instead of bit reversing the entire trace which can be expensive

There's already a TODO to remove the cloning of the whole trace for this calculation. Only the relevant columns should be passed here after that change so this will stay the same. I edited to TODO to be clearer.

[shaharsamocha7]

Reviewed 3 of 3 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alonh5 and @spapinistarkware)

---

crates/prover/src/examples/wide_fibonacci/mod.rs line 153 at r6 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

I need to turn the trace order into circle domain and bit reverse for the same reason above.

ok but we can also apply the todo here in order to reverse only the columns that are relevant for the lookup, right?

[spapinistarkware]

Reviewed 3 of 7 files at r2, 1 of 3 files at r5, 1 of 2 files at r6, 3 of 3 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @alonh5)

[alonh5]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7)

---

crates/prover/src/examples/wide_fibonacci/mod.rs line 153 at r6 (raw file):

Previously, shaharsamocha7 wrote…

ok but we can also apply the todo here in order to reverse only the columns that are relevant for the lookup, right?

First of all this is a test just making sure you noticed. Secondly in the actual trace I don't think we can only change the order of some of the columns because that would mess up the constraints in the same row.

[shaharsamocha7]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @alonh5)

[alonh5]

Merge activity

• Jul 2, 8:47 AM EDT: @alonh5 started a stack merge that includes this pull request via Graphite.
• Jul 2, 8:47 AM EDT: @alonh5 merged this pull request with Graphite.