Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement GkrOps for CPU backend #621

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

andrewmilson
Copy link
Contributor

@andrewmilson andrewmilson commented May 8, 2024

This change is Reviewable

@andrewmilson andrewmilson force-pushed the 05-08-Implement_GkrOps_for_CPU_backend branch from 3c5a3fc to a014862 Compare May 19, 2024 14:35
@andrewmilson andrewmilson force-pushed the 05-08-Implement_GkrOps_for_CPU_backend branch 2 times, most recently from 66728bd to 753aad2 Compare May 24, 2024 19:14
Copy link
Contributor Author

@andrewmilson andrewmilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


crates/prover/src/core/backend/cpu/lookups/gkr.rs line 17 at r1 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

I would prefer to allocate this with capacity.
consider implementing this iteratively instead, if it helps.

Done.

@andrewmilson andrewmilson force-pushed the andrew/dev/lookups/gkr branch 3 times, most recently from 8cd2fd4 to aaad529 Compare June 13, 2024 03:40
@andrewmilson andrewmilson force-pushed the 05-08-Implement_GkrOps_for_CPU_backend branch from 753aad2 to fbd5d23 Compare June 13, 2024 03:40
@andrewmilson andrewmilson force-pushed the 05-08-Implement_GkrOps_for_CPU_backend branch from fbd5d23 to 5705279 Compare June 13, 2024 04:09
Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @andrewmilson and @spapinistarkware)


crates/prover/src/core/backend/cpu/lookups/gkr.rs line 3 at r2 (raw file):

use crate::core::backend::CpuBackend;
use crate::core::fields::qm31::SecureField;
use crate::core::lookups::gkr::GkrOps;

Shouldn't it be gkr_prover? can you rebase?

Code quote:

use crate::core::lookups::gkr::GkrOps;

crates/prover/src/core/backend/cpu/lookups/gkr.rs line 48 at r2 (raw file):

        let y = [7, 3].map(|v| BaseField::from(v).into());

        let eq_evals = CpuBackend::gen_eq_evals(&y, one);

Consider to test it with v != one.

Code quote:

let eq_evals = CpuBackend::gen_eq_evals(&y, one);

@andrewmilson andrewmilson force-pushed the 05-08-Implement_GkrOps_for_CPU_backend branch 2 times, most recently from 38cef08 to 778d86c Compare June 13, 2024 15:45
Copy link
Contributor Author

@andrewmilson andrewmilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @shaharsamocha7 and @spapinistarkware)


crates/prover/src/core/backend/cpu/lookups/gkr.rs line 3 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Shouldn't it be gkr_prover? can you rebase?

Thanks, updated

@andrewmilson andrewmilson force-pushed the 05-08-Implement_GkrOps_for_CPU_backend branch 2 times, most recently from 158add9 to 40a1a18 Compare June 19, 2024 03:10
Copy link
Collaborator

@spapinistarkware spapinistarkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

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

Base automatically changed from andrew/dev/lookups/gkr to dev June 20, 2024 13:41
Copy link
Collaborator

@shaharsamocha7 shaharsamocha7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @andrewmilson)

@andrewmilson andrewmilson force-pushed the 05-08-Implement_GkrOps_for_CPU_backend branch from 40a1a18 to 8a53b70 Compare June 20, 2024 13:50
@andrewmilson andrewmilson merged commit 3d2728c into dev Jun 20, 2024
14 checks passed
@andrewmilson andrewmilson deleted the 05-08-Implement_GkrOps_for_CPU_backend branch June 20, 2024 15:01
andrewmilson added a commit that referenced this pull request Jun 26, 2024
…D_backend' into 06-16-add_lookup_final_boundary_constraints

* origin/05-24-Implement_LogupOps_for_SIMD_backend:
  Implement LogupOps for SIMD backend
  Implement GrandProductOps for SIMD backend
  Implement GkrOps for SIMD backend
  Implement MleOps for SIMD backend
  Add GKR implementation of Logup lookups
  Add GKR implementation of Grand Product lookups
  Implement GkrOps for CPU backend (#621)
  Add batch GKR prover and verifier (#581)
  add parallelization feature for Merkle tree (#658)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants