Simplify Bus Interaction #2047
Simplify Bus Interaction #2047
Conversation
georgwiese
force-pushed
the
simplify-bus-interaction
branch
from
3229bc4
to
662045d
Compare
| use std::protocols::lookup::unpack_lookup_constraint; | ||
| use std::math::fp2::Fp2; | ||
| use std::constraints::to_phantom_lookup; | ||
|
|
There was a problem hiding this comment.
Why not keep the simple lookup function that just calls send and receive?
There was a problem hiding this comment.
Because we'd want to do the receive in the callee machine and the send in the caller machine. Otherwise, the added columns would be in the wrong namespace.
There was a problem hiding this comment.
But namespaces will not matter later on anyway...
There was a problem hiding this comment.
But they do now :) I think this a simple thing to fix later.
Also, this API is more flexible: Instead of generating one send and receive per lookup, we can only generate one send per caller (same as now) and one receive per receiver (instead of one per caller).
There was a problem hiding this comment.
How do they matter? Only wrt degree, or is there something else?
There was a problem hiding this comment.
We split by namespace in our split_pil function, used by the composite backend and Plonky3 backend to split the columns (and constraints) into machines, so we can prove them even when they a have different number of rows. I think this will go away once we have proof objects.
If we had one lookup function that was called in the machine linking to the callee (just like our current ASM linker adds the native lookup constraint to the caller), the multiplicity column would end up in the wrong namespace, which would lead to an error in the split_pil function, because it doesn't expect constraints to reference columns in multiple namespaces.
georgwiese
force-pushed
the
simplify-bus-interaction
branch
from
5519119
to
32f40d0
Compare
|
Rebased onto |
| // Currently, witgen fails if the block machine has just enough rows to | ||
| // fit all the blocks, so let's not have a call in the last row. | ||
| col fixed sel = [1, 1, 1, 1, 1, 1, 1, 0]; |
There was a problem hiding this comment.
This can be removed after #2070 is merged.
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
Fixes #1823 This PR cleans up our `bus_interaction` function and all functions that depend on it, so that columns and challenges are added when needed, instead of being passed by the caller. I also changed the tests a bit: I removed `test_data/std/bus_{permutation,lookup}_via_challenges{_ext?}.asm` and instead added more realistic tests: - `test_data/std/bus_lookup.asm`: Is a fixed lookup, completely analogous to `lookup_via_challenges`. - `test_data/std/bus_permutation.asm`: Uses a permutation to connect a block machine.
Fixes #1823
This PR cleans up our
bus_interactionfunction and all functions that depend on it, so that columns and challenges are added when needed, instead of being passed by the caller.I also changed the tests a bit: I removed
test_data/std/bus_{permutation,lookup}_via_challenges{_ext?}.asmand instead added more realistic tests:test_data/std/bus_lookup.asm: Is a fixed lookup, completely analogous tolookup_via_challenges.test_data/std/bus_permutation.asm: Uses a permutation to connect a block machine.