Air with dyn component

[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.36%. Comparing base (b806150) to head (13efd90).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #487      +/-   ##
==========================================
- Coverage   95.41%   95.36%   -0.06%     
==========================================
  Files          58       58              
  Lines        8906     8795     -111     
  Branches     8906     8795     -111     
==========================================
- Hits         8498     8387     -111     
  Misses        359      359              
  Partials       49       49              

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

[alonh5]

Reviewed 1 of 4 files at r1, 4 of 5 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @shaharsamocha7 and @spapinistarkware)

---

src/core/air/component_visitors.rs line 139 at r3 (raw file):

            .map(|component| {
                let n_columns = component.trace_log_degree_bounds().len();
                let columns = polynomials.iter().take(n_columns).collect();

I think this is needed to mutate self.polynomials.

Suggestion:

(&mut self.polynomials)

---

src/core/prover/mod.rs line 175 at r3 (raw file):

    commitment_domains.push(CanonicCoset::new(
        air.composition_log_degree_bound() + LOG_BLOWUP_FACTOR,
    ));

Why not add this inside the air method?

Code quote:

    commitment_domains.push(CanonicCoset::new(
        air.composition_log_degree_bound() + LOG_BLOWUP_FACTOR,
    ));

[shaharsamocha7]

Reviewed 1 of 4 files at r1, 3 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @spapinistarkware)

---

src/core/air/component_visitors.rs line 1 at r3 (raw file):

use core::slice;

Maybe this file should be renamed?

Code quote:

use core::slice;

---

src/core/air/component_visitors.rs line 65 at r3 (raw file):

    }

    fn mask_points(

Unit test for this function?
I don't see where you move the mask by the component offset

Code quote:

fn mask_points(

---

src/core/air/component_visitors.rs line 100 at r3 (raw file):

    /// Returns the log degree bounds of the quotient polynomials in descending order.
    fn quotient_log_bounds(&self) -> Vec<CirclePolyDegreeBound> {

Can we have a unit test to this function?
I don't understand how it is works in case you have multiple components

Code quote:

fn quotient_log_bounds(&self) -> Vec<CirclePolyDegreeBound> {

[spapinistarkware]

Reviewable status: 4 of 8 files reviewed, 4 unresolved discussions (waiting on @alonh5 and @shaharsamocha7)

---

src/core/prover/mod.rs line 175 at r3 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Why not add this inside the air method?

It's the only place that will need it. Why move it? It's more explicit this wait.

---

src/core/air/component_visitors.rs line 65 at r3 (raw file):

Previously, shaharsamocha7 wrote…

Unit test for this function?
I don't see where you move the mask by the component offset

What's a component offset?
I didnt change this function, just translated to a loop.

---

src/core/air/component_visitors.rs line 100 at r3 (raw file):

Previously, shaharsamocha7 wrote…

Can we have a unit test to this function?
I don't understand how it is works in case you have multiple components

I didnt change it. Adding tests sounds nice, but not for this PR.

---

src/core/air/component_visitors.rs line 139 at r3 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

I think this is needed to mutate self.polynomials.

Done.

[alonh5]

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

---

src/core/air/mod.rs line 18 at r4 (raw file):

pub use air_ext::AirExt;

/// Aritair_exte Representation (AIR).

Restore.

---

src/core/prover/mod.rs line 175 at r3 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

It's the only place that will need it. Why move it? It's more explicit this wait.

It seems more natural, the verify function is long as is. But your call, it's not crucial.

[spapinistarkware]

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

---

src/core/air/mod.rs line 18 at r4 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Restore.

Done.

---

src/core/prover/mod.rs line 175 at r3 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

It seems more natural, the verify function is long as is. But your call, it's not crucial.

It'll get shorter upstack.

[alonh5]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @shaharsamocha7)

[spapinistarkware]

Merge activity

• Mar 19, 11:10 AM EDT: @spapinistarkware started a stack merge that includes this pull request via Graphite.
• Mar 19, 11:11 AM EDT: @spapinistarkware merged this pull request with Graphite.