Relax restriction to a single looked table for CTL #1575

matthiasgoergens · 2024-04-22T02:42:57Z

EDIT: it's perhaps better to get #1577 in first, and then I rebase this PR.

In our use of plonky2, we have a few instances where we need both several looked and several looking tables. Think of these lookups as many-to-many multiplexers, or a kind of bus. Looking tables can be thought of as pushing values with an obligation to process them onto the bus, looked tables can be seen as pulling values to discharge that obligation.

My first attempt changed from looked_table to looked_tables, but then I noticed that thanks to logup, we don't need this complication: looked_tables are just looking_tables with negative multiplicities.

Thus we can get a simpler system by just removing looked_table completely, and giving people tools to negate the multiplicities of their lookup tables.

We also preserve the old CrossTableLookup::new, which applies the negation to the passed looked_table automatically to keep the API as compatible as possible.

This PR only touches cross table lookups. We could do the same for the intra-table lookups in starky/src/lookup.rs. But that's for a separate PR.

matthiasgoergens · 2024-04-22T02:47:51Z

starky/src/cross_table_lookup.rs

-        },
-    ) in cross_table_lookups.iter().enumerate()
-    {
+    for (index, CrossTableLookup { looking_tables }) in cross_table_lookups.iter().enumerate() {
        // Get elements looking into `looked_table` that are not associated to any STARK.
        let extra_sum_vec: &[F] = ctl_extra_looking_sums


ctl_extra_looking_sums are now indexed the same as cross_table_lookups, not like looked tables.

matthiasgoergens · 2024-04-22T02:48:57Z

starky/src/cross_table_lookup.rs

@@ -1041,26 +971,21 @@ pub mod debug_utils {
        }
    }

-    fn check_ctl<F: Field>(
+    fn check_ctl<F: PrimeField64>(


I'm not sure these functions are actually in use anywhere? They don't seem to have been updated to reflect the greater capabilities of logup vs the old product argument.

They are being used on the zk_evm side here: prover.rs

In our use of plonky2, we have a few instances where we need both several looked and several looking tables. Think of these lookups as many-to-many multiplexers, or a kind of bus. Looking tables can be thought of as pushing values with an obligation to process them onto the bus, looked tables can be seen as pulling values to discharge that obligation. My first attempt changed from `looked_table` to `looked_tables`, but then I noticed that thanks to logup, we don't need this complication: looked_tables are just looking_tables with negative multiplicities. Thus we can get a simpler system by just removing `looked_table` completely, and giving people tools to negate the multiplicities of their lookup tables. We also preserve the old `CrossTableLookup::new`, which applies the negation to the passed `looked_table` automatically to keep the API as compatible as possible.

LindaGuiga

Thank you @matthiasgoergens , it's a good idea!
But I tried running this PR against the integration tests in zk_evm (for example add11_yml), and CTLs and Lookups failed. So I was wondering whether this was also the case for you or if maybe I had messed something up when testing?

Nashtare · 2024-04-24T14:35:11Z

Thank you @matthiasgoergens , it's a good idea!
But I tried running this PR against the integration tests in zk_evm (for example add11_yml), and CTLs and Lookups failed. So I was wondering whether this was also the case for you or if maybe I had messed something up when testing?

This branch is based on an older version of #1577 which was broken (and that @matthiasgoergens now fixed). After rebasing this should go away I believe.

LindaGuiga · 2024-04-24T15:13:59Z

Thank you @matthiasgoergens , it's a good idea!
But I tried running this PR against the integration tests in zk_evm (for example add11_yml), and CTLs and Lookups failed. So I was wondering whether this was also the case for you or if maybe I had messed something up when testing?

This branch is based on an older version of #1577 which was broken (and that @matthiasgoergens now fixed). After rebasing this should go away I believe.

Indeed, it works with those changes, thanks!

LindaGuiga

The changes look great (once it includes the fix from #1577 ), thank you @matthiasgoergens !

matthiasgoergens · 2024-04-26T02:45:56Z

Could you please tell me how to run the zk_evm tests? Is it just 'cargo test' in there (after fixing the dependencies to point at my plonky2 branch)? Or something more involved? Thanks!

Btw, do you have a way to run some benchmarks for zk_evm? I mean, I found some micro-benchmarks in the repo, but I'm looking for something more substantial that covers more of the system (but hopefully doesn't run for longer than a minute or perhaps five at most for one iteration).

That's because I have some ideas for speeding up plonky2 that work for our system, but I want to make sure they don't make life worse for you.

(And chances are, if it helps two unrelated system, it's probably a good idea in general.)

Nashtare · 2024-04-26T15:19:05Z

@matthiasgoergens For testing, yes you can just run cargo --release --test --package evm_arithmetization within zk_evm repo: https://github.com/0xpolygonzero/zk_evm/.

For benchmarking purposes, we usually would do it against real-life examples, with zero-bin: https://github.com/0xpolygonzero/zero-bin/. There you can use the stdio mode to pass a block trace as input, and generate an entire proof for the block. If you need an access endpoint / block traces to play with, feel free to reach me in DM.

field/src/types.rs

starky/src/cross_table_lookup.rs

Nashtare · 2024-06-04T17:27:35Z

@matthiasgoergens Could you address the few comments / rebase with latest main so we can get this into the next release?

…les-upstream

matthiasgoergens · 2024-06-05T11:12:43Z

@matthiasgoergens Could you address the few comments / rebase with latest main so we can get this into the next release?

Please see the new version.

matthiasgoergens · 2024-06-05T11:17:05Z

starky/src/cross_table_lookup.rs

+                .map(|table| table.table)
+                .counts()
+                .into_iter()
+                .sorted()


We sort to keep the debug output deterministic.

Nashtare · 2024-06-05T21:28:37Z

starky/src/cross_table_lookup.rs

+        log::debug!(
+            "Processing CTL for {:?}",
+            looking_tables
+                .iter()
+                .map(|table| table.table)
+                .counts()
+                .into_iter()
+                .sorted()
+                .collect::<Vec<_>>()
+        );


At this point I think I may prefer just specifying for which table the CTL is, i.e.

Suggested change

log::debug!(

"Processing CTL for {:?}",

looking_tables

.iter()

.map(|table| table.table)

.counts()

.into_iter()

.sorted()

.collect::<Vec<_>>()

);

log::debug!(

"Processing CTL for STARK table {:?}",

looking_tables

.last().

.expect("We always have a looked table.")

.table

);

or otherwise removing it altogether. The current display doesn't carry much meaning (especially with the sorting which kind of hides which table is being looked at). For example this is our zkEVM output with this implem:

[DEBUG starky::cross_table_lookup] Processing CTL for [(0, 1), (2, 1)] [DEBUG starky::cross_table_lookup] Processing CTL for [(1, 1), (2, 4)] [DEBUG starky::cross_table_lookup] Processing CTL for [(2, 1), (4, 1)] [DEBUG starky::cross_table_lookup] Processing CTL for [(3, 1), (4, 1)] [DEBUG starky::cross_table_lookup] Processing CTL for [(3, 1), (4, 1)] [DEBUG starky::cross_table_lookup] Processing CTL for [(2, 1), (4, 5), (5, 1)] [DEBUG starky::cross_table_lookup] Processing CTL for [(1, 32), (2, 7), (4, 136), (6, 1)]

gio256 · 2024-06-18T19:53:16Z

@matthiasgoergens thanks for this PR.

Can I be of any help getting this across the line? Happy to take it from here or just get it caught up to main.

Nashtare · 2024-08-06T12:47:31Z

Closing for inactivity.
@gio256 feel free to pick it up if you want to / have some free time.

matthiasgoergens requested review from muursh, wborgeaud and Nashtare as code owners April 22, 2024 02:42

matthiasgoergens force-pushed the matthias/looked-tables-upstream branch from 2bebb8f to d1e57ba Compare April 22, 2024 02:46

matthiasgoergens commented Apr 22, 2024

View reviewed changes

matthiasgoergens changed the title ~~Relax restriction to a single looked table~~ Relax restriction to a single looked table for CTL Apr 22, 2024

matthiasgoergens mentioned this pull request Apr 22, 2024

Relax restriction to a single looked table for CTL 0xmozak/plonky2#90

Open

matthiasgoergens force-pushed the matthias/looked-tables-upstream branch 2 times, most recently from a274126 to 2e21697 Compare April 22, 2024 04:43

matthiasgoergens force-pushed the matthias/looked-tables-upstream branch from 2e21697 to 6ef8270 Compare April 22, 2024 04:48

Remove restriction to binary-only multiplicities

7538119

matthiasgoergens mentioned this pull request Apr 22, 2024

Remove restriction to binary-only multiplicities #1577

Merged

Nashtare requested a review from LindaGuiga April 23, 2024 13:28

LindaGuiga reviewed Apr 24, 2024

View reviewed changes

matthiasgoergens mentioned this pull request Apr 25, 2024

Fix verify_cross_table_lookups with no ctl_extra_looking_sums #1584

Merged

Nashtare reviewed May 1, 2024

View reviewed changes

field/src/types.rs Outdated Show resolved Hide resolved

starky/src/cross_table_lookup.rs Outdated Show resolved Hide resolved

starky/src/cross_table_lookup.rs Outdated Show resolved Hide resolved

matthiasgoergens added 2 commits June 5, 2024 18:47

Use assert_zero

6afb2ca

Merge remote-tracking branch 'upstream/main' into matthias/looked-tab…

e1c2acb

…les-upstream

matthiasgoergens force-pushed the matthias/looked-tables-upstream branch from 5ef7b30 to e1c2acb Compare June 5, 2024 11:06

Split negative/positive values at ORDER/2

0575518

Keep count

681dda5

matthiasgoergens commented Jun 5, 2024

View reviewed changes

Nashtare added this to the Cleanups and Misc. milestone Jun 5, 2024

Nashtare reviewed Jun 5, 2024

View reviewed changes

gio256 added a commit to gio256/rizzo that referenced this pull request Jun 18, 2024

Add todo reference to 0xPolygonZero/plonky2#1575

f1e5721

Nashtare modified the milestones: Misc., System strengthening Jul 12, 2024

Nashtare closed this Aug 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Relax restriction to a single looked table for CTL #1575

Relax restriction to a single looked table for CTL #1575

matthiasgoergens commented Apr 22, 2024 •

edited

Loading

matthiasgoergens Apr 22, 2024

matthiasgoergens Apr 22, 2024

Nashtare Apr 22, 2024

LindaGuiga left a comment

Nashtare commented Apr 24, 2024

LindaGuiga commented Apr 24, 2024

LindaGuiga left a comment

matthiasgoergens commented Apr 26, 2024

Nashtare commented Apr 26, 2024

Nashtare commented Jun 4, 2024

matthiasgoergens commented Jun 5, 2024

matthiasgoergens Jun 5, 2024

Nashtare Jun 5, 2024 •

edited

Loading

gio256 commented Jun 18, 2024

Nashtare commented Aug 6, 2024

Relax restriction to a single looked table for CTL #1575

Relax restriction to a single looked table for CTL #1575

Conversation

matthiasgoergens commented Apr 22, 2024 • edited Loading

matthiasgoergens Apr 22, 2024

Choose a reason for hiding this comment

matthiasgoergens Apr 22, 2024

Choose a reason for hiding this comment

Nashtare Apr 22, 2024

Choose a reason for hiding this comment

LindaGuiga left a comment

Choose a reason for hiding this comment

Nashtare commented Apr 24, 2024

LindaGuiga commented Apr 24, 2024

LindaGuiga left a comment

Choose a reason for hiding this comment

matthiasgoergens commented Apr 26, 2024

Nashtare commented Apr 26, 2024

Nashtare commented Jun 4, 2024

matthiasgoergens commented Jun 5, 2024

matthiasgoergens Jun 5, 2024

Choose a reason for hiding this comment

Nashtare Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

gio256 commented Jun 18, 2024

Nashtare commented Aug 6, 2024

matthiasgoergens commented Apr 22, 2024 •

edited

Loading

Nashtare Jun 5, 2024 •

edited

Loading