LinePoly backend

[spapinistarkware]

This change is 

[spapinistarkware]

Current dependencies on/for this PR:

• LinePoly backend #350 : 2 dependent PRs (#352 , #391 ) 👈
• dev

This stack of pull requests is managed by Graphite.

[shaharsamocha7]

Reviewed 7 of 8 files at r1, all commit messages.
Reviewable status: 7 of 8 files reviewed, 5 unresolved discussions (waiting on @spapinistarkware)

---

src/core/fri.rs line 195 at r1 (raw file):

        let last_layer_poly = LinePoly::from_ordered_coefficients(coeffs);
        let x: &Column<B, SecureField> = &last_layer_poly.coeffs;

Rename

Code quote:

x

---

src/core/backend/cpu/mod.rs line 28 at r1 (raw file):

    }
    fn to_vec(&self) -> Vec<F> {
        self.clone()

Do we have to use clone here? It could be very costly.

Code quote:

self.clone()

---

src/core/poly/line.rs line 107 at r1 (raw file):

    ///
    /// The coefficients are stored in bit-reversed order.
    pub coeffs: Column<B, F>,

Why public? Andrew won't like it :)

Code quote:

pub 

---

src/core/poly/line.rs line 164 at r1 (raw file):

pub struct LineEvaluation<B: LinePolyOps<F>, F: Field, EvalOrder = NaturalOrder> {
    /// Evaluations of a univariate polynomial on `domain`.
    pub values: Column<B, F>,

Did you change that to match the CircleEvaluation?
Can we make this change together with changing the evals arg in new()?

Code quote:

pub values: Column<B, F>,

---

src/core/poly/line.rs line 176 at r1 (raw file):

    fn deref(&self) -> &[F] {
        &self.coeffs
    }

Why did you remove that?
Can we do it in a different pr? It requires a lot of unrelated changes.

Code quote:

    fn deref(&self) -> &[F] {
        &self.coeffs
    }

[spapinistarkware]

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

---

src/core/fri.rs line 195 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Rename

Done.

---

src/core/backend/cpu/mod.rs line 28 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Do we have to use clone here? It could be very costly.

If you are using to_vec(), you are going to pay. It should be used only for very small stuff or tests.
Most backends will have to copy anyway.

---

src/core/poly/line.rs line 107 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Why public? Andrew won't like it :)

Will go away next PR.

---

src/core/poly/line.rs line 164 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Did you change that to match the CircleEvaluation?
Can we make this change together with changing the evals arg in new()?

evals sounds like multiple LineEvalaution. I thougth value is better.

---

src/core/poly/line.rs line 176 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Why did you remove that?
Can we do it in a different pr? It requires a lot of unrelated changes.

We can't, unfortunately. What will this return? We can't return &Column, since it's &Vec, not &[]. We could add ColumnRef object, but it's a lot more unecessary code.

[shaharsamocha7]

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

---

src/core/fri.rs line 877 at r2 (raw file):

/// Panics if there are less than two evaluations.
pub fn fold_line<F: ExtensionOf<BaseField>>(
    evals: &LineEvaluation<B, F, BitReversedOrder>,

Why is it called evals?
Can you fix it? also in documentation

Code quote:

evals

[shaharsamocha7]

Reviewed 1 of 8 files at r1, 6 of 7 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @spapinistarkware)

---

src/core/fri.rs line 764 at r4 (raw file):

    fn new(evaluation: LineEvaluation<B, SecureField, BitReversedOrder>) -> Self {
        // TODO: Commit on slice.
        let merkle_tree = MerkleTree::commit(vec![evaluation.values.clone()]);

Suggestion:

to_vec()

---

src/core/backend/mod.rs line 27 at r4 (raw file):

    type Column: Column<F>;
    fn bit_reverse_column(column: Self::Column) -> Self::Column;
}

Is this trait belongs to this file?

Code quote:

pub trait FieldOps<F: Field> {
    type Column: Column<F>;
    fn bit_reverse_column(column: Self::Column) -> Self::Column;
}

[spapinistarkware]

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

---

src/core/fri.rs line 764 at r4 (raw file):

    fn new(evaluation: LineEvaluation<B, SecureField, BitReversedOrder>) -> Self {
        // TODO: Commit on slice.
        let merkle_tree = MerkleTree::commit(vec![evaluation.values.clone()]);

Done.

---

src/core/backend/mod.rs line 27 at r4 (raw file):

Previously, shaharsamocha7 wrote…

Is this trait belongs to this file?

I thought it is. Do you have another suggestion?

[shaharsamocha7]

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

---

src/core/backend/mod.rs line 27 at r4 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

I thought it is. Do you have another suggestion?

Maybe near the field trait?
The FriOps, PolyOps are sitting near the relevant code.

[shaharsamocha7]

Reviewed 1 of 4 files at r6, 8 of 9 files at r7, all commit messages.
Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @spapinistarkware)

---

src/core/backend/cpu/mod.rs line 40 at r7 (raw file):

pub type CPUCircleEvaluation<F, EvalOrder = NaturalOrder> =
    CircleEvaluation<CPUBackend, F, EvalOrder>;
pub type CPULineEvaluation<F, EvalOrder = NaturalOrder> = LineEvaluation<CPUBackend, F, EvalOrder>;

Add a TODO to remove the EvalOrder here?

Code quote:

valOrder = NaturalOrder

---

src/core/fields/mod.rs line 16 at r7 (raw file):

    fn bit_reverse_column(column: Self::Column) -> Self::Column;
}
pub type Col<B, F> = <B as FieldOps<F>>::Column;

Maybe FieldColumn?

Code quote:

Col

[spapinistarkware]

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

---

src/core/backend/cpu/mod.rs line 40 at r7 (raw file):

Previously, shaharsamocha7 wrote…

Add a TODO to remove the EvalOrder here?

Done.

---

src/core/fields/mod.rs line 16 at r7 (raw file):

Previously, shaharsamocha7 wrote…

Maybe FieldColumn?

Done.

[shaharsamocha7]

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