-
Notifications
You must be signed in to change notification settings - Fork 174
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
refactor: refactor range checker #949
Conversation
a91a60e
to
6e3ac9a
Compare
ffcee6c
to
83c1482
Compare
aede21a
to
f2328dc
Compare
627f144
to
2278e57
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.
Looks right! Thank you! This is not a full review yet - but I left a few small comments inline.
b16cbd0
to
0cacdda
Compare
b2a0487
to
5bbbee3
Compare
- Simplifies the range checker. - Removes one selector column in the main trace and one running product column in the auxiliary trace
…raints - removes one main and one auxiliary trace columns - removes constraints for the 8 bit section and remove the selector column
5bbbee3
to
d04275d
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.
Looks good! Thank you! I left a few minor comments inline.
f564d22
to
92cdd43
Compare
92cdd43
to
3aa5dae
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.
Looks good! Thank you!
Describe your changes
Addressing #395. This PR simplifies the range checker and removes the 8-bit section of the range checker. The new design works as follows:
Hence, the constraint to check the transition between two rows in the range checker becomes:
This construction reduces the minimum trace length to 64 from the current 1024 and removes 1 column from the main trace and 1 column from the auxiliary trace.
Integration tests run time reduced by ~42% after this PR.
Edit: I haven't included the excalidraw image zip file for the updated range checker image in this. I'll add that before we merge this.
Checklist before requesting a review
next
according to naming convention.