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

refactor: refactor range checker #949

Merged
merged 6 commits into from
Jun 30, 2023
Merged

refactor: refactor range checker #949

merged 6 commits into from
Jun 30, 2023

Conversation

tohrnii
Copy link
Contributor

@tohrnii tohrnii commented Jun 20, 2023

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:

  1. The values to be range checked are sorted.
  2. The difference between two consecutive values to be range checked can be expressed as a linear combination of powers of 3 less than or equal to $3^7$, i.e.,

$$ \Delta n = \sum_{i=0}^{7} x_i \cdot 3^i $$

  1. Then bridge rows are added in between the values to be range checked. For each $3^i$ in the above equation, $x_i$ number of bridge rows are added. However, if the sum of $x_i$ from $i=0$ to $i=7$ is 0 or 1, no bridge rows are required since the next row contains the value to be range checked.
    Hence, the constraint to check the transition between two rows in the range checker becomes:

$$ \Delta v \cdot (\Delta v - 1) \cdot (\Delta v - 3) \cdot (\Delta v - 9) \cdot (\Delta v - 27) \cdot (\Delta v - 81) \cdot (\Delta v - 243) \cdot (\Delta v - 729) \cdot (\Delta v - 2187) = 0 $$

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.

  • remove 8 bit portion of the range checker
  • update trace
  • update constraints
  • update tests
  • refactor and cleanup
  • update docs
  • update images

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

  • Repo forked and branch created from next according to naming convention.
  • Commit messages and codestyle follow conventions.
  • Relevant issues are linked in the PR description.
  • Tests added for new functionality.
  • Documentation/comments updated according to changes.

@tohrnii tohrnii force-pushed the tohrnii-rangechecker branch 9 times, most recently from a91a60e to 6e3ac9a Compare June 21, 2023 08:19
@tohrnii tohrnii force-pushed the tohrnii-rangechecker branch 3 times, most recently from ffcee6c to 83c1482 Compare June 28, 2023 12:24
@tohrnii tohrnii changed the base branch from next to grjte-hasher-row-addr June 28, 2023 12:24
@bobbinth bobbinth force-pushed the grjte-hasher-row-addr branch from aede21a to f2328dc Compare June 28, 2023 18:25
Base automatically changed from grjte-hasher-row-addr to next June 28, 2023 18:52
@tohrnii tohrnii force-pushed the tohrnii-rangechecker branch 6 times, most recently from 627f144 to 2278e57 Compare June 29, 2023 01:56
@tohrnii tohrnii requested review from grjte and bobbinth June 29, 2023 01:59
@tohrnii
Copy link
Contributor Author

tohrnii commented Jun 29, 2023

@bobbinth @grjte I haven't updated the images yet. I'll update them if the rest of the changes looks correct.

Copy link
Contributor

@bobbinth bobbinth left a 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.

@tohrnii tohrnii force-pushed the tohrnii-rangechecker branch from b16cbd0 to 0cacdda Compare June 29, 2023 10:15
@tohrnii tohrnii force-pushed the tohrnii-rangechecker branch 2 times, most recently from b2a0487 to 5bbbee3 Compare June 29, 2023 17:16
@tohrnii tohrnii requested a review from bobbinth June 29, 2023 17:17
@tohrnii tohrnii marked this pull request as ready for review June 29, 2023 17:17
tohrnii added 4 commits June 29, 2023 16:49
- 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
@bobbinth bobbinth force-pushed the tohrnii-rangechecker branch from 5bbbee3 to d04275d Compare June 29, 2023 23:50
Copy link
Contributor

@bobbinth bobbinth left a 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.

@tohrnii tohrnii force-pushed the tohrnii-rangechecker branch from f564d22 to 92cdd43 Compare June 30, 2023 04:14
@tohrnii tohrnii requested a review from bobbinth June 30, 2023 04:15
@tohrnii tohrnii force-pushed the tohrnii-rangechecker branch from 92cdd43 to 3aa5dae Compare June 30, 2023 06:35
Copy link
Contributor

@bobbinth bobbinth left a 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!

@tohrnii tohrnii merged commit ca9a330 into next Jun 30, 2023
@tohrnii tohrnii deleted the tohrnii-rangechecker branch June 30, 2023 14:26
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.

2 participants