feat: add vector equality chip #34
Conversation
There was a problem hiding this comment.
Not done yet, but is_equal and is_zero should be restructured to follow the structure in #35
Namely, IsEqual and IsZero airs should implement AirConfig, SubAir, LocalTraceInstructions. They do not need to implement SubAirWithInteractions since they have no interactions.
In particular, avoid copy/paste of code whenever possible. IsEqual should be using the subair trait implementations of IsZero, and IsEqualVec should use the other two subairs as much as possible.
|
btw the git best practice is to "resolve conversations" once you have pushed commits to address them. you're encouraged to make small commits and push often |
…type into feat/is-equal-vec
|
I thought we wanted to use more efficient chips; I believe rewriting IsEqual and IsEqualVec from scratch is more efficient than using the subairs IsZero and IsEqual. |
…to iszeroair, renamed generate_trace
…rewrote trace generation following subchip
…ed chip.rs noops, is-equal constraint optimization
There was a problem hiding this comment.
There were a bunch of review comments that were not addressed - maybe you need to expand hidden comments. I fixed a bunch of them in 6149b6f @cocohearts please review that commit so you know what changes (and also some rust best practices).
Going to approve this to unblock other stuff but you should still:
- fix is_equal_vec test as mentioned in comment
- in a separate PR, add
test_caseto clean up redundant test code.
* adding is_zero chip with corresponding tests * adjusting is_zero to follow guidelines: request, vector, etc * wrote is_equal chip directly, without interaction * completed all of is_equal_vec with tests * feat: add `SubAir` trait * feat: add `SubAirLocalTraceInstructions` for row-wise trace generation * feat: add `SubAirWithInteractions` trait * chore: fix clippy * adding is_zero chip with corresponding tests * adjusting is_zero to follow guidelines: request, vector, etc * wrote is_equal chip directly, without interaction * completed all of is_equal_vec with tests * for is_zero: added airconfig trait, combine iszerochip and iszeroair to iszeroair, renamed generate_trace * moved airconfig declaration to mod, imp generate_trace_row * rewrote generate_trace using generate_trace_row * rewrote tests * remove pi * rewrite is_zero constraints for efficiency * added subair eval for iszerochip * de-vectorizing request to follow xor_bits * reuse is_zero in trace generation * chore: cleanup, rename * in is_equal: combined air, chip struc, added chip.rs, expanded cols, rewrote trace generation following subchip * wrapped air builder, rewrote tests without pi * chore: fix imports * added is_equal_vec air specs, optimized constraints and trace generation * update is-equal-vec air docstring * wrap out subair and subtrace for is-vec-cols * feat: update SubAir associated types to separate Io and Aux * added new io/aux col spec with new structs, used alignedborrow, removed chip.rs noops, is-equal constraint optimization * removing extraneous test comments * u32->F for is-zero * chore: chips only take size, not input data * feat: automated rand negative test * chore: cleanup code * feat: add is_zero.flatten() * feat: vector tests for is_zero * chore: code, comment cleanup * chore: comments for air.rs, rename to_vec to flatten * chore: clean code --------- Co-authored-by: Alex Zhao <alexzhao@intrinsictech.com> Co-authored-by: Jonathan Wang <31040440+jonathanpwang@users.noreply.github.com>
IsEqualVecChipthat takes two 2D vectors ofu32and checks for Boolean equality for each corresponding vector, outputting 0 or 1IsZeroChipand scalarIsEqualChip