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

feat: single page index scan with predicate #58

Merged
merged 63 commits into from
Jun 18, 2024
Merged

Conversation

bfan05
Copy link
Contributor

@bfan05 bfan05 commented Jun 11, 2024

single page index scan for predicates of the form index OP x, where OP is one of <, <=, =, >=, > and x is a private input

bfan05 added 30 commits May 29, 2024 16:49
@bfan05 bfan05 changed the title [wip] feat: single page index scan for less than predicate feat: single page index scan for less than predicate Jun 12, 2024
@bfan05 bfan05 changed the title feat: single page index scan for less than predicate feat: single page index scan with predicate Jun 12, 2024
@bfan05 bfan05 requested a review from jonathanpwang June 12, 2024 20:15
@jonathanpwang
Copy link
Contributor

Resolves INT-1530

Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is overall correct, but the code is hard to read and verbose due to the match statements. It would be better if the code can be abridged to reflect that: you check predicate by using one or both of is_less_than and is_equal, and its mostly a question of which one(s) to use and what arguments to put in the subair eval.

Tips that may help:

  • move match statements to functions of the enum itself
  • you can use Option to have match return None for irrelevant branches

@bfan05 bfan05 requested a review from jonathanpwang June 17, 2024 21:57
Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall much improved. Left some minor comments, and also on the constraint side question about why when_transition is used.

Otherwise LGTM

@bfan05 bfan05 merged commit bedfdab into main Jun 18, 2024
3 checks passed
@bfan05 bfan05 deleted the feat/page-index-scan branch June 18, 2024 16:20
luffykai pushed a commit that referenced this pull request Dec 13, 2024
* feat: sorted_limbs chip checking each limb less than limb_bits bits

* feat: completed sorted_limbs chip with tests

* feat: SortedLimbsChip with LessThan subchip

* feat: less_than subchip refactored

* feat: rename SortedLimbsChip to AssertSortedChip and write LessThanChip tests

* chore: change name of assert sorted chip

* chore: fix names in tests for AssertSortedChip

* chore: address comments

* chore: cleanup

* chore: change MAX from generic to instance field for LessThanChip and AssertSortedChip

* feat: IsLessThanChip to compare two numbers

* feat: IsLessThanTuple subchip for different limb_bits

* feat: IsLessThanTupleChip subchip in AssertSortedChip

* chore: address comments first pass

* chore: refactor AssertSorted, IsEqual, IsLessThan, and IsLessThanTuple chips

* chore: address comments

* chore: eliminate high dim poly from IsLessThanTupleChip

* chore: fix tests

* chore: address comments for AssertSortedChip

* chore: cleanup AssertSorted

* chore: cleanup

* chore: include roundtrip flatten and from_slice tests

* feat: flatten and from_slice for IO and Aux columns

* create files

* chore: begin PageIndexScanChip

* feat: prototype

* feat: rename Chip to AirBridge

* feat: single page index scan chip for less than predicate

* feat: partitioned main

* chore: cleanup index scan for less than predicate

* feat: x as public value

* feat: page index scan with comparator enum

* feat: page index scan for greater than predicate

* feat: page index scan for equal to predicates

* feat: page index scan for less than or equal to predicate

* feat: page index scan for greater than or equal to predicate

* chore: cleanup some branches

* chore: refactor code to reduce repetition

* chore: cleanup PageIndexScanInputChip

* chore: fix test

* merge IsEqualVec columns

* remove range_max from PageIndexScan chips

* chore: address comments

* feat: use FinalPage

* chore: refactor index_scan_test

* chore: address comments
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