Move SecureColumn

[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

Attention: Patch coverage is 70.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 95.43%. Comparing base (4dc88e9) to head (2f3df09).

Files	Patch %	Lines
src/core/fields/secure.rs	70.00%	6 Missing ⚠️

Additional details and impacted files

@@                           Coverage Diff                           @@
##           spapini/03-13-Use_only_canonic_domains     #478   +/-   ##
=======================================================================
  Coverage                                   95.43%   95.43%           
=======================================================================
  Files                                          59       59           
  Lines                                        8699     8699           
  Branches                                     8699     8699           
=======================================================================
  Hits                                         8302     8302           
  Misses                                        347      347           
  Partials                                       50       50           

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

[spapinistarkware]

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

a discussion (no related file):

Previously, shaharsamocha7 wrote…

Is fields/secure.rs the correct place for that logic?
Seems like this is a different field from qm31.

Also the logic here fits under poly dir, WDYT?

It's not just for poly. SecureColumn is for any column of secure fields.
It's the same field as qm31, but it's more of gadgets on top of it.
Not sure where is best to put it

---

[shaharsamocha7]

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

---

src/core/fields/secure.rs line 12 at r2 (raw file):

pub const SECURE_EXTENSION_DEGREE: usize =
    <SecureField as ExtensionOf<BaseField>>::EXTENSION_DEGREE;

You have a const here

stwo/src/core/fields/qm31.rs

Line 11 in e3167b0

pub const SECURE_FIELD_EXTENSION_DEGREE: usize = 4;

Maybe those can be combined?

Code quote:

pub const SECURE_EXTENSION_DEGREE: usize =
    <SecureField as ExtensionOf<BaseField>>::EXTENSION_DEGREE;

---

src/core/fields/secure.rs line 14 at r2 (raw file):

    <SecureField as ExtensionOf<BaseField>>::EXTENSION_DEGREE;

pub struct SecureColumn<B: Backend> {

Document

Code quote:

pub struct SecureColumn<B: Backend> {

---

src/core/fields/secure.rs line 15 at r2 (raw file):

pub struct SecureColumn<B: Backend> {
    pub cols: [Col<B, BaseField>; SECURE_EXTENSION_DEGREE],

Suggestion:

pub columns: [Col<B, BaseField>; SECURE_EXTENSION_DEGREE],

---

src/core/fields/secure.rs line 45 at r2 (raw file):

}

pub struct SecureCirclePoly(pub [CPUCirclePoly; SECURE_EXTENSION_DEGREE]);

I think that the previous place of that part of this file was better.

Code quote:

pub struct SecureCirclePoly(pub [CPUCirclePoly; SECURE_EXTENSION_DEGREE]);

---

src/core/fields/secure.rs line 71 at r2 (raw file):

}

pub fn combine_secure_value(value: [SecureField; SECURE_EXTENSION_DEGREE]) -> SecureField {

This function should be documented.

Code quote:

pub fn combine_secure_value(value: [SecureField; SECURE_EXTENSION_DEGREE]) -> SecureField {

[spapinistarkware]

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

---

src/core/fields/secure.rs line 12 at r2 (raw file):

Previously, shaharsamocha7 wrote…

You have a const here

stwo/src/core/fields/qm31.rs

Line 11 in e3167b0

pub const SECURE_FIELD_EXTENSION_DEGREE: usize = 4;

Maybe those can be combined?

Done.

---

src/core/fields/secure.rs line 14 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Document

Done.

---

src/core/fields/secure.rs line 15 at r2 (raw file):

pub struct SecureColumn<B: Backend> {
    pub cols: [Col<B, BaseField>; SECURE_EXTENSION_DEGREE],

Done.

---

src/core/fields/secure.rs line 45 at r2 (raw file):

Previously, shaharsamocha7 wrote…

I think that the previous place of that part of this file was better.

Done.

---

src/core/fields/secure.rs line 71 at r2 (raw file):

Previously, shaharsamocha7 wrote…

This function should be documented.

Done.

[shaharsamocha7]

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

---

src/core/channel.rs line 126 at r4 (raw file):

        let felts: [BaseField; FELTS_PER_HASH] = self.draw_base_felts();
        SecureField::from_m31_array(
            felts[..<SecureField as ExtensionOf<BaseField>>::EXTENSION_DEGREE]

Why not using the const that you defined there?

Suggestion:

felts[..SECURE_EXTENSION_DEGREE]

---

src/core/poly/circle/secure_poly.rs line 28 at r4 (raw file):

    /// Evaluates the polynomial at a point, given evaluations of its composing base field
    /// polynomials' evaluations at that point.

Suggestion:

    /// Evaluates the polynomial at a point, given evaluations of its composing base field
    /// polynomials at that point.

---

src/core/poly/circle/secure_poly.rs line 29 at r4 (raw file):

    /// Evaluates the polynomial at a point, given evaluations of its composing base field
    /// polynomials' evaluations at that point.
    pub fn eval_from_partial_evals(value: [SecureField; SECURE_EXTENSION_DEGREE]) -> SecureField {

?

Suggestion:

evals

[spapinistarkware]

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

---

src/core/channel.rs line 126 at r4 (raw file):

Previously, shaharsamocha7 wrote…

Why not using the const that you defined there?

Done.

---

src/core/poly/circle/secure_poly.rs line 28 at r4 (raw file):

    /// Evaluates the polynomial at a point, given evaluations of its composing base field
    /// polynomials' evaluations at that point.

Done.

[shaharsamocha7]

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

[spapinistarkware]

Merge activity

• Mar 21, 7:23 AM EDT: @spapinistarkware started a stack merge that includes this pull request via Graphite.
• Mar 21, 7:25 AM EDT: Graphite rebased this pull request as part of a merge.
• Mar 21, 7:26 AM EDT: @spapinistarkware merged this pull request with Graphite.