Merge trace writer and trace generator.

[alonh5]

This change is 

[alonh5]

• Merge trace writer and trace generator. #672 : 2 dependent PRs (#674 , #675 ) 👈
• 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

Attention: Patch coverage is 8.00000% with 23 lines in your changes missing coverage. Please review.

Project coverage is 90.09%. Comparing base (91a2833) to head (2ce2998).

Files	Patch %	Lines
crates/prover/src/trace_generation/registry.rs	0.00%	12 Missing ⚠️
crates/prover/src/core/prover/mod.rs	15.38%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #672      +/-   ##
==========================================
- Coverage   90.29%   90.09%   -0.21%     
==========================================
  Files          75       75              
  Lines       10030    10053      +23     
  Branches    10030    10053      +23     
==========================================
  Hits         9057     9057              
- Misses        892      915      +23     
  Partials       81       81              

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

[shaharsamocha7]

Reviewed 9 of 10 files at r1, all commit messages.
Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @alonh5)

---

crates/prover/src/examples/fibonacci/component.rs line 142 at r1 (raw file):

        _registry: &mut ComponentRegistry,
    ) -> ColumnVec<CircleEvaluation<CpuBackend, BaseField, BitReversedOrder>> {
        vec![]

Do we want to implement write_trace here?
Could be done in another pr

What would it require to use this write_trace function instead of the current implementation?

Code quote:

vec![]

---

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

        _registry: &mut ComponentRegistry,
    ) -> ColumnVec<CircleEvaluation<CpuBackend, BaseField, BitReversedOrder>> {
        vec![]

same

Code quote:

        vec![]

[alonh5]

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

---

crates/prover/src/examples/fibonacci/component.rs line 142 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Do we want to implement write_trace here?
Could be done in another pr

What would it require to use this write_trace function instead of the current implementation?

Yes I'll start working on it in a new PR.

---

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

Previously, shaharsamocha7 wrote…

same

same

[shaharsamocha7]

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

---

crates/prover/src/trace_generation/mod.rs line 19 at r1 (raw file):

// A trait to generate a a trace.
// Generates the trace given a list of inputs collects inputs for subcomponents.
pub trait TraceGenerator<B: Backend> {

Do you think we should do this renaming? because I see that we also have AirTraceGenertor.

If so, can you do it in another pr?

Suggestion:

pub trait ComponentTraceGenerator<B: Backend> {

---

crates/prover/src/trace_generation/mod.rs line 45 at r1 (raw file):

pub trait AirTraceVerifier {
    fn interaction_elements(&self, channel: &mut Blake2sChannel) -> InteractionElements;
}

I think that this trait should be in the core library (it is used also by the verifier)

Code quote:

pub trait AirTraceVerifier {
    fn interaction_elements(&self, channel: &mut Blake2sChannel) -> InteractionElements;
}

[alonh5]

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

---

crates/prover/src/trace_generation/mod.rs line 19 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Do you think we should do this renaming? because I see that we also have AirTraceGenertor.

If so, can you do it in another pr?

Done in this PR.

---

crates/prover/src/trace_generation/mod.rs line 45 at r1 (raw file):

Previously, shaharsamocha7 wrote…

I think that this trait should be in the core library (it is used also by the verifier)

Technically the verify function receives an Air which is also an AirTraceVerifier, but I think it's better to leave it close to the AirTraceGenerator. Do you think that's okay?

[shaharsamocha7]

Reviewable status: 1 of 10 files reviewed, 1 unresolved discussion (waiting on @alonh5)

---

crates/prover/src/trace_generation/mod.rs line 45 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Technically the verify function receives an Air which is also an AirTraceVerifier, but I think it's better to leave it close to the AirTraceGenerator. Do you think that's okay?

Yes, but that raises another question?
Why isn't the function interaction_elements just part of the Air trait?

[alonh5]

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

---

crates/prover/src/trace_generation/mod.rs line 45 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Yes, but that raises another question?
Why isn't the function interaction_elements just part of the Air trait?

That's because for the prover we need that function before we know all the components, in the trace generation phase. If we want it to be part of Air then AirTraceGenerator would also have to be an Air and therefore know it's components (we've talked about this before).

[shaharsamocha7]

Reviewed 1 of 10 files at r1, 8 of 8 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @alonh5)

---

crates/prover/src/trace_generation/mod.rs line 45 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

That's because for the prover we need that function before we know all the components, in the trace generation phase. If we want it to be part of Air then AirTraceGenerator would also have to be an Air and therefore know it's components (we've talked about this before).

right

[shaharsamocha7]

Reviewed 8 of 8 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @alonh5)

---

crates/prover/src/trace_generation/mod.rs line 37 at r3 (raw file):

    ) -> ColumnVec<CircleEvaluation<B, BaseField, BitReversedOrder>>;

    fn write_interaction_trace(

Add documentation

Code quote:

fn write_interaction_trace(

[alonh5]

Reviewable status: 3 of 11 files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7)

---

crates/prover/src/trace_generation/mod.rs line 37 at r3 (raw file):

Previously, shaharsamocha7 wrote…

Add documentation

Done.

[shaharsamocha7]

Reviewed 9 of 9 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @alonh5)

[alonh5]

Merge activity

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