WideFib test with AVX Backend

[spapinistarkware]

This change is 

[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 👈
• Make prove() generic in Backend #490 : 1 other dependent PR (#491 )
• CommitmentSchemeProver generic in backend #489
• Fri AVX ops #563
• AVX quotients #488
• Remove old merkle #531 : 1 other dependent PR (#486 )
• FRI using simple merkle #530
• Dumb down FRI #529
• avx merkle #528
• simple merkle benchmark #527
• Use simple merkle tree #526
• merkle intermediate layers #548
• 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 94.04%. Comparing base (90e675a) to head (4b151b8).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #492      +/-   ##
==========================================
+ Coverage   93.34%   94.04%   +0.70%     
==========================================
  Files          65       65              
  Lines        8299     8302       +3     
  Branches     8299     8302       +3     
==========================================
+ Hits         7747     7808      +61     
+ Misses        490      432      -58     
  Partials       62       62              

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

[shaharsamocha7]

Reviewed 1 of 13 files at r2.
Reviewable status: 1 of 13 files reviewed, 1 unresolved discussion (waiting on @spapinistarkware)

---

src/core/air/accumulation.rs line 169 at r2 (raw file):

pub struct ColumnAccumulator<'a, B: Backend> {
    pub random_coeff_pow: SecureField,
    pub col: &'a mut SecureColumn<B>,

I changed this code. you probably have some conflicts :(

Code quote:

pub struct ColumnAccumulator<'a, B: Backend> {
    pub random_coeff_pow: SecureField,
    pub col: &'a mut SecureColumn<B>,

[shaharsamocha7]

Reviewed 2 of 13 files at r2.
Reviewable status: 3 of 13 files reviewed, 6 unresolved discussions (waiting on @spapinistarkware)

---

src/core/backend/avx512/blake2s.rs line 28 at r2 (raw file):

    ) -> Vec<Blake2sHash> {
        // Pad prev_layer if too small.
        if log_size < 4 {

Suggestion:

VECS_LOG_SIZE

---

src/examples/wide_fibonacci/avx.rs line 95 at r2 (raw file):

            let mut b = trace_eval[1].data[vec_row];
            #[allow(clippy::needless_range_loop)]
            for i in 0..(N_COLS - 2) {

Can you add the boundary constraint?

Code quote:

            #[allow(clippy::needless_range_loop)]
            for i in 0..(N_COLS - 2) {

---

src/examples/wide_fibonacci/avx.rs line 104 at r2 (raw file):

            // Denominator.
            // TODO(spapini): Optimized this, for the small number of columns case.

Is this the coset_vanishing function?
Don't we already have one?

Suggestion:

// TODO(spapini): Move to PolyOps? and optimized for the small number of columns case.

---

src/examples/wide_fibonacci/avx.rs line 118 at r2 (raw file):

            accum.col.set_packed(
                vec_row,
                accum.col.packed_at(vec_row) * PackedQM31::broadcast(accum.random_coeff_pow)

Where the denomenator is used?

---

src/examples/wide_fibonacci/avx.rs line 158 at r2 (raw file):

    #[test]
    fn test_avx_wide_fib_prove() {
        // TODO(spapini): Increase to 20, to get 1GB of trace.

You can do these now, no?

Code quote:

 // TODO(spapini): Increase to 20, to get 1GB of trace.

[spapinistarkware]

Reviewable status: 1 of 13 files reviewed, 5 unresolved discussions (waiting on @shaharsamocha7)

---

src/core/air/accumulation.rs line 169 at r2 (raw file):

Previously, shaharsamocha7 wrote…

I changed this code. you probably have some conflicts :(

Done.

---

src/examples/wide_fibonacci/avx.rs line 95 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Can you add the boundary constraint?

Done.

---

src/examples/wide_fibonacci/avx.rs line 104 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Is this the coset_vanishing function?
Don't we already have one?

Sorry, leftovers.

---

src/examples/wide_fibonacci/avx.rs line 118 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Where the denomenator is used?

Done.

---

src/examples/wide_fibonacci/avx.rs line 158 at r2 (raw file):

Previously, shaharsamocha7 wrote…

You can do these now, no?

When I test with AVX flags not on release, it's slow. I use it for my tests, so I'd rather only do this when I benchmark.

[shaharsamocha7]

Reviewed 2 of 13 files at r2, 7 of 10 files at r3, all commit messages.
Reviewable status: 10 of 13 files reviewed, 4 unresolved discussions (waiting on @spapinistarkware)

---

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

        Self {
            random_coeff_powers: generate_secure_powers(random_coeff, total_columns),
            sub_accumulations: (0..(max_log_size + 1)).map(|_| None).collect(),

For efficiency?

Code quote:

(0..(max_log_size + 1)).map(|_| None).collect(),

---

src/examples/wide_fibonacci/constraint_eval.rs line 436 at r3 (raw file):

        evaluation_accumulator: &mut PointEvaluationAccumulator,
    ) {
        let constraint_zero_domain = Coset::subgroup(self.log_size);

Why it is not a CanonicCoset?

Code quote:

Coset::subgroup(self.log_size);

---

src/examples/wide_fibonacci/mod.rs line 50 at r3 (raw file):

            QM31::from_u32_unchecked(1, 2, 3, 4),
            wide_fib.log_size + 1,
            Component::<CPUBackend>::n_constraints(&wide_fib),

A question:
Why this change is good?

Code quote:

Component::<CPUBackend>::n_constraints(&wide_fib)

---

src/examples/wide_fibonacci/avx.rs line 158 at r2 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

When I test with AVX flags not on release, it's slow. I use it for my tests, so I'd rather only do this when I benchmark.

so this TODO is to add benchmark? can you change that?

[spapinistarkware]

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

---

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

Previously, shaharsamocha7 wrote…

For efficiency?

Because small sizes are not supported in AVX backend.

---

src/examples/wide_fibonacci/constraint_eval.rs line 436 at r3 (raw file):

Previously, shaharsamocha7 wrote…

Why it is not a CanonicCoset?

Good point. I wonder why the test didnt catch it. We should inverstigate this.

---

src/examples/wide_fibonacci/mod.rs line 50 at r3 (raw file):

Previously, shaharsamocha7 wrote…

A question:
Why this change is good?

Not good. Necessary.
We should separate Component to a backedn dependenc one and a non backend dependent one.
Currently, when we do .n_constraints(), it doesnt know which impl to use (for which of the backends)

[shaharsamocha7]

Reviewable status: 8 of 13 files reviewed, 2 unresolved discussions (waiting on @spapinistarkware)

---

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

Previously, spapinistarkware (Shahar Papini) wrote…

Because small sizes are not supported in AVX backend.

I don't see the connection.
It seems that you don't initialize unused accumulations, no?

---

src/examples/wide_fibonacci/constraint_eval.rs line 436 at r3 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

Good point. I wonder why the test didnt catch it. We should inverstigate this.

It works for both CanonicCoset and subgroup?
That's really strange.

[spapinistarkware]

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

---

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

Previously, shaharsamocha7 wrote…

I don't see the connection.
It seems that you don't initialize unused accumulations, no?

The issue was that I interpolate each of these sub accumulations. And I am not allowed to do it on small ones right now

[spapinistarkware]

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

---

src/examples/wide_fibonacci/constraint_eval.rs line 436 at r3 (raw file):

Previously, shaharsamocha7 wrote…

It works for both CanonicCoset and subgroup?
That's really strange.

I agree. We should inverstigate this.

[shaharsamocha7]

Reviewed 3 of 10 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @spapinistarkware)

---

src/examples/wide_fibonacci/constraint_eval.rs line 436 at r3 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

I agree. We should inverstigate this.

Solution here is now clear, it worked since the poly was constant?

---

src/examples/wide_fibonacci/avx.rs line 161 at r4 (raw file):

    #[test]
    fn test_avx_wide_fib_prove() {

Need to fix this test before

Code quote:

fn test_avx_wide_fib_prove()

[spapinistarkware]

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

---

src/examples/wide_fibonacci/constraint_eval.rs line 436 at r3 (raw file):

Previously, shaharsamocha7 wrote…

Solution here is now clear, it worked since the poly was constant?

Yep

---

src/examples/wide_fibonacci/avx.rs line 161 at r4 (raw file):

Previously, shaharsamocha7 wrote…

Need to fix this test before

Waiting on you now (mask points change)

[spapinistarkware]

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

---

src/examples/wide_fibonacci/avx.rs line 161 at r4 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

Waiting on you now (mask points change)

Done.

[shaharsamocha7]

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