-
Notifications
You must be signed in to change notification settings - Fork 12
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
generate enabler column #333
generate enabler column #333
Conversation
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
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: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7)
stwo_cairo_prover/crates/prover/src/components/utils.rs
line 87 at r1 (raw file):
/// The padding column is a MultiplicityColumn with the first `padding_offset` elements set to 1 and /// the rest set to 0. pub fn gen_padding_column(log_size: usize, padding_offset: usize) -> MultiplicityColumn {
I don't think this should go through the MultiplicityColumn logic
can you just generate a vector of simd elements? I think it should also be faster (calloc, memwrite, fix one vector in the middle)
Previously, ohad-starkware (Ohad) wrote…
take only offset, the size is always next_power_of_2 right? |
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: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @ohad-starkware)
stwo_cairo_prover/crates/prover/src/components/utils.rs
line 87 at r1 (raw file):
Previously, ohad-starkware (Ohad) wrote…
take only offset, the size is always next_power_of_2 right?
if not, logsize u32
Do you think it matters in terms of efficiency?
I just wanted the simplest implementation that would be clear what it is.
Using MultiplicityColumn to generate the padding column is the most clear to me, WDYT?
748e324
to
4886ee3
Compare
Previously, shaharsamocha7 wrote…
it doesn't matter |
Previously, ohad-starkware (Ohad) wrote…
it doesn't matter in terms of efficiency* |
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: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @ohad-starkware)
stwo_cairo_prover/crates/prover/src/components/utils.rs
line 87 at r1 (raw file):
Previously, ohad-starkware (Ohad) wrote…
it doesn't matter in terms of efficiency*
so you don't think of the padding column as a multiplicity column?
I'm willing to change the impl, do you have a better name for this function?
Previously, shaharsamocha7 wrote…
gen_enabler_column |
This change is