allocating batch inverse

[ohad-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[ohad-starkware]

• use parallel batch inverse in logup #987
• parallel batch inverse #984
• specific simd batch inverse functions #985
• chunked batch inverse #983
• allocating batch inverse #977 👈 (View in Graphite)
• remove not used FieldOps trait #969
• dev

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

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 97.72727% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.32%. Comparing base (d05215c) to head (b0d17cb).
Report is 2 commits behind head on dev.

Files with missing lines	Patch %	Lines
...es/prover/src/core/backend/simd/very_packed_m31.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #977      +/-   ##
==========================================
- Coverage   92.38%   92.32%   -0.06%     
==========================================
  Files         105      105              
  Lines       14232    14228       -4     
  Branches    14232    14228       -4     
==========================================
- Hits        13148    13136      -12     
- Misses       1010     1018       +8     
  Partials       74       74              

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

[shaharsamocha7]

Reviewed all commit messages.
Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @ohad-starkware)

---

crates/prover/src/core/backend/cpu/circle.rs line 175 at r2 (raw file):

            .zip(itwiddles.array_chunks_mut::<CHUNK_SIZE>())
            .for_each(|(src, dst)| {
                batch_inverse_in_place(src, dst);

I don't think we want the in_place suffix here.

Code quote:

batch_inverse_in_place

[ohad-starkware]

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

---

crates/prover/src/core/backend/cpu/circle.rs line 175 at r2 (raw file):

Previously, shaharsamocha7 wrote…

I don't think we want the in_place suffix here.

here or in general?
this can be made to use the chunked batch inverse after it's merged up the stack
I do need to expose both interfaces because we are batch inversing constant sized array so a -> vec interface is not sufficient

[shaharsamocha7]

Reviewed 5 of 9 files at r2.
Reviewable status: 5 of 9 files reviewed, 4 unresolved discussions (waiting on @ohad-starkware)

---

crates/prover/src/core/fields/mod.rs line 39 at r2 (raw file):

/// Assumes dst is initialized and of the same length as column.
fn batch_inverse_classic<T: FieldExpOps>(column: &[T], dst: &mut [T]) {

Consider to state somehow that this is relevant for the in_place variant

Suggestion:

/// Assumes dst is initialized and of the same length as column.
fn batch_inverse_in_place_slow<T: FieldExpOps>(column: &[T], dst: &mut [T]) {

---

crates/prover/src/core/fields/mod.rs line 99 at r2 (raw file):

// TODO(Ohad): chunks, parallelize.
pub fn batch_inverse<F: FieldExpOps>(column: &[F]) -> Vec<F> {

Can we test this instead of directly batch_inverse_in_place?

Code quote:

pub fn batch_inverse<F: FieldExpOps>(column: &[F]) -> Vec<F> {

---

crates/prover/src/core/backend/cpu/mod.rs line 109 at r2 (raw file):

    }

    // TODO(Ohad): remove.

why?

Code quote:

// TODO(Ohad): remove.

---

crates/prover/src/core/backend/cpu/quotients.rs line 135 at r2 (raw file):

    }

    CM31::invert_many(&denominators)

rename as discussed

Code quote:

CM31::invert_many

[ohad-starkware]

Reviewable status: 5 of 9 files reviewed, 4 unresolved discussions (waiting on @ohad-starkware and @shaharsamocha7)

---

crates/prover/src/core/backend/cpu/mod.rs line 109 at r2 (raw file):

Previously, shaharsamocha7 wrote…

why?

weird location for this test

---

crates/prover/src/core/backend/cpu/quotients.rs line 135 at r2 (raw file):

Previously, shaharsamocha7 wrote…

rename as discussed

Done.

---

crates/prover/src/core/fields/mod.rs line 39 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Consider to state somehow that this is relevant for the in_place variant

it's a subroutine (not pub)

---

crates/prover/src/core/fields/mod.rs line 99 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Can we test this instead of directly batch_inverse_in_place?

done.

[shaharsamocha7]

Reviewed 1 of 9 files at r2, 6 of 6 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ohad-starkware)