-
Notifications
You must be signed in to change notification settings - Fork 44
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 SIMD backend #630
base: 05-15-Implement_MleOps_for_SIMD_backend
Are you sure you want to change the base?
Implement GkrOps for SIMD backend #630
Conversation
e5c33f3
to
075e1e0
Compare
f7837b1
to
7878fb0
Compare
075e1e0
to
8ced162
Compare
7878fb0
to
13b3e3c
Compare
8ced162
to
a8ba532
Compare
13b3e3c
to
ed308eb
Compare
a8ba532
to
fa110ae
Compare
ed308eb
to
0e58c2e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 6 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @andrewmilson)
fa110ae
to
db14e69
Compare
0e58c2e
to
3b03377
Compare
db14e69
to
df114f5
Compare
3b03377
to
6b36881
Compare
df114f5
to
f103b6e
Compare
6b36881
to
0874a3e
Compare
f103b6e
to
c4ddfdd
Compare
0874a3e
to
5674f67
Compare
c4ddfdd
to
5555a3e
Compare
5674f67
to
df87de3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 6 files at r1, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @andrewmilson)
crates/prover/src/core/backend/simd/lookups/gkr.rs
line 21 at r3 (raw file):
} // Start DP with CPU backend to prevent dealing with instances smaller than a SIMD vector.
what's DP?
Code quote:
Start DP with
crates/prover/src/core/backend/simd/lookups/gkr.rs
line 22 at r3 (raw file):
// Start DP with CPU backend to prevent dealing with instances smaller than a SIMD vector. let (y_initial, y_rem) = y.split_last_chunk::<{ LOG_N_LANES as usize }>().unwrap();
y_initial is the last 4 elements in the vector, need another name IMO
or maybe comment that we pass on y in reverse order?
Code quote:
let (y_initial, y_rem) = y.split_last_chunk::<{ LOG_N_LANES as usize }>().unwrap();
crates/prover/src/core/backend/simd/lookups/gkr.rs
line 70 at r3 (raw file):
#[test] fn gen_eq_evals_matches_cpu() {
Can you also add a test where y.len() < LOG_N_LANES
?
Code quote:
fn gen_eq_evals_matches_cpu() {
5555a3e
to
8ceac89
Compare
df87de3
to
f19de9e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 5 of 6 files reviewed, 3 unresolved discussions (waiting on @shaharsamocha7 and @spapinistarkware)
crates/prover/src/core/backend/simd/lookups/gkr.rs
line 21 at r3 (raw file):
Previously, shaharsamocha7 wrote…
what's DP?
Dynamic Programming
crates/prover/src/core/backend/simd/lookups/gkr.rs
line 22 at r3 (raw file):
Previously, shaharsamocha7 wrote…
y_initial is the last 4 elements in the vector, need another name IMO
or maybe comment that we pass on y in reverse order?
Initial in the sense that it's the y values that create the initial values for the algorithm but I see how It's confusing. Changed to y_last_chunk
. WDYT?
crates/prover/src/core/backend/simd/lookups/gkr.rs
line 70 at r3 (raw file):
Previously, shaharsamocha7 wrote…
Can you also add a test where
y.len() < LOG_N_LANES
?
Added
8ceac89
to
e175251
Compare
f19de9e
to
7188de2
Compare
e175251
to
c51f096
Compare
7188de2
to
4968f10
Compare
This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)