-
Notifications
You must be signed in to change notification settings - Fork 394
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
feat: byte multiplicity channel #800
Conversation
core/src/alu/lt/mod.rs
Outdated
@@ -116,6 +119,8 @@ impl<F: PrimeField32> MachineAir<F> for LtChip { | |||
let c = event.c.to_le_bytes(); | |||
|
|||
cols.shard = F::from_canonical_u32(event.shard); | |||
cols.channel = F::from_canonical_u32(event.channel); | |||
cols.channel = F::from_canonical_u32(event.channel); |
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.
dup
core/src/alu/mul/mod.rs
Outdated
cols.channel = F::from_canonical_u32(event.channel); | ||
cols.channel = F::from_canonical_u32(event.channel); |
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.
cols.channel = F::from_canonical_u32(event.channel); | |
cols.channel = F::from_canonical_u32(event.channel); | |
cols.channel = F::from_canonical_u32(event.channel); |
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.
generally LGTM. Added a few nit columns.
core/src/cpu/columns/channel.rs
Outdated
// Accumulate the reconstructed channel. | ||
reconstruct_channel += selector.into() * AB::Expr::from_canonical_u32(i as u32); | ||
} | ||
// Asser that the reconstructed channel is the same as the channel. |
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.
nit: typo
core/src/runtime/mod.rs
Outdated
self.state.channel += 1; | ||
self.state.channel %= NUM_BYTE_LOOKUP_CHANNELS; |
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.
nit: Maybe collapse the two lines into one.
self.state.channel += 1; | |
self.state.channel %= NUM_BYTE_LOOKUP_CHANNELS; | |
self.state.channel = (self.state.channel + 1) % NUM_BYTE_LOOKUP_CHANNELS; |
core/src/runtime/state.rs
Outdated
@@ -25,6 +25,9 @@ pub struct ExecutionState { | |||
/// executed in this shard. | |||
pub clk: u32, | |||
|
|||
/// The channel alternates between 0 and 1, used to controll byte lookup multiplicity. |
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.
nit: Consider updating the column for NUM_BYTE_LOOKUP_CHANNELS
.
@@ -23,6 +23,9 @@ use crate::bytes::trace::NUM_ROWS; | |||
/// The number of different byte operations. | |||
pub const NUM_BYTE_OPS: usize = 9; | |||
|
|||
/// The number of different byte lookup channels. | |||
pub const NUM_BYTE_LOOKUP_CHANNELS: u32 = 4; |
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.
nit: Should this be changed to 4 to minimize the increase in columns on the CPU table (via the channel selector columns)?
No description provided.