Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

SubCircuit halo2 stats #1763

Merged
merged 8 commits into from
Feb 22, 2024
Merged

SubCircuit halo2 stats #1763

merged 8 commits into from
Feb 22, 2024

Conversation

ed255
Copy link
Member

@ed255 ed255 commented Feb 9, 2024 •
edited
Loading

Description

Add a new command to the stats binary that reports halo2 circuit stats for each circuit, and the supercircuit.

Issue Link

Resolve #1645

Type of change

  • New feature (non-breaking change which adds functionality)

Contents

  • Added new module in stats that records halo2 circuit stats from the configured ConstraintSystem, based on Scroll's code from https://github.com/scroll-tech/zkevm-circuits/blob/7d9bc181953cfc6e7baf82ff0ce651281fd70a8a/zkevm-circuits/src/util.rs#L294
    • The collection of stats works in two phases, first it records per-circuit stats and then total stats (to get super circuit stats). The per-circuit stats works by configuring all the shared tables into the same ConstraintSystem (and obtaining stats via deltas), and then configuring each sub-circuit starting from the shared tables ConstraintSystems each time.
    • In order to separate the shared column configuration from the sub-circuit and constraints configuration, I had to extract the column definition of the BinaryNumberChip into BinaryNumberBits so that the CopyTable can be constructed without any constraint.
  • Added peak memory estimation based on @han0110's analysis

Results

These results are for k = 26. At the current worst-case estimation of At 0.0139 gas/row this gives us 2^26 * 0.0139 = ~900k gas. For 30M gas we would need 33 chunks like that.

circuit constraints rots min/max(rots) fix_cols sels advs perms lookups degree mem_gb
tx_table 0 0 0/0 1 0 4 0 0 3 58
wd_table 0 0 0/0 0 0 5 0 0 3 56
rw_table 0 0 0/0 0 0 14 0 0 3 110
mpt_table 0 0 0/0 0 0 12 0 0 3 98
bytecode_table 0 0 0/0 0 0 6 0 0 3 62
block_table 0 0 0/0 2 0 2 0 0 3 54
copy_table 0 0 0/0 1 0 12 0 0 3 106
exp_table 0 0 0/0 1 0 5 0 0 3 64
keccak_table 0 0 0/0 0 0 5 0 0 3 56
sig_table 0 0 0/0 1 0 9 0 0 3 88
u8_table 0 0 0/0 1 0 0 0 0 3 34
u10_table 0 0 0/0 1 0 0 0 0 3 34
u16_table 0 0 0/0 1 0 0 0 0 3 34
keccak 2523 105 -89/207 18 0 198 0 123 4 3000
pi 21 2 0/1 3 7 10 15 3 9 754
tx 2 2 0/1 13 4 6 9 6 6 780
bytecode 23 2 0/1 5 0 6 0 2 8 342
copy 34 3 0/2 1 1 14 0 12 9 466
state 203 3 -1/1 3 0 49 0 36 10 2224
exp 44 11 0/10 0 1 10 0 0 5 142
evm 42318 21 0/20 5 3 131 3 53 7 2976
super 45168 106 -89/207 57 16 498 25 235 10 21736

PD: Yes, the estimation is 21TB of memory for a super circuit proof :( Nevertheless there are various things we can do to improve this.

@ed255 ed255 changed the title Feature/circuit stats SubCircuit halo2 stats Feb 9, 2024
@github-actions github-actions bot added crate-zkevm-circuits Issues related to the zkevm-circuits workspace member crate-gadgets Issues related to the gadgets workspace member labels Feb 9, 2024
@ed255 ed255 force-pushed the feature/circuit-stats branch from c391b0e to df2a1bb Compare February 12, 2024 13:39
@ed255 ed255 marked this pull request as draft February 13, 2024 09:10
@ed255
Copy link
Member Author

ed255 commented Feb 13, 2024

Marking as draft because it seems I broke the CopyCircuit or CopyTable.

@ed255 ed255 mentioned this pull request Feb 15, 2024
Open
@ed255 ed255 force-pushed the feature/circuit-stats branch 2 times, most recently from deb3cbc to 01c5133 Compare February 16, 2024 10:08
@ed255 ed255 marked this pull request as ready for review February 16, 2024 10:08
@ed255 ed255 force-pushed the feature/circuit-stats branch from e87e531 to 2905d4e Compare February 16, 2024 11:09
@github-actions github-actions bot added the crate-integration-tests Issues related to the integration-tests workspace member label Feb 16, 2024
@KimiWu123 KimiWu123 self-requested a review February 16, 2024 12:09
@miha-stopar miha-stopar self-requested a review February 16, 2024 15:00
KimiWu123
KimiWu123 reviewed Feb 17, 2024
View reviewed changes
Copy link
Contributor

@KimiWu123 KimiWu123 left a comment

Choose a reason for hiding this comment

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

First round pass. Will take a deeper look at halo2_stats.rs.

zkevm-circuits/src/super_circuit.rs Outdated Show resolved Hide resolved
KimiWu123
KimiWu123 reviewed Feb 19, 2024
View reviewed changes
Copy link
Contributor

@KimiWu123 KimiWu123 left a comment

Choose a reason for hiding this comment

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

Overall looks good! Have a question about formula and implementation mismatch.

zkevm-circuits/src/bin/stats/halo2_stats.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/bin/stats/halo2_stats.rs Show resolved Hide resolved
zkevm-circuits/src/bin/stats/halo2_stats.rs Outdated Show resolved Hide resolved
zkevm-circuits/src/bin/stats/halo2_stats.rs Show resolved Hide resolved
@ed255 ed255 force-pushed the feature/circuit-stats branch from edf2afd to 7020216 Compare February 19, 2024 16:30
ChihChengLiang
ChihChengLiang reviewed Feb 22, 2024
View reviewed changes
Copy link
Collaborator

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

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

I don't intend to do a full review. Please exclude me from the final approval.
I just want to give a quick comment on the Deref.

gadgets/src/binary_number.rs Outdated Show resolved Hide resolved
han0110
han0110 approved these changes Feb 22, 2024
View reviewed changes
Copy link
Collaborator

@han0110 han0110 left a comment

Choose a reason for hiding this comment

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

LGTM!

ed255 added 8 commits February 22, 2024 11:42
@ed255
feat: add super circuit stats
b1425b0
@ed255
feat: extract bit columns from BinaryNumberChip
c65f4ad 
Extract the bit columns from BinaryNumberChip into BinaryNumberBits so
that the columns can be used in a table without associated constraints.
This is useful for the CopyTable to be used in unit tests outside of the
CopyCircuit (without needing the BinaryNumberChip constraints).
@ed255
feat: use peak memory estimation from Han
405384c
@ed255
feat(supercircuit): use real challenge when mock-challenges is disabled
e007276
@ed255
fix: address feedback from @KimiWu123
cd2988c
@ed255
fix(zkevm-circuits/stats): add SigTable
1722253
@ed255
fix: address feedback from @miha-stopar and @han0110
f40887a
@ed255
feat: implement Deref for BinaryNumberBits
9563fdc
@ed255 ed255 force-pushed the feature/circuit-stats branch from d9fc61b to 9563fdc Compare February 22, 2024 11:42
@miha-stopar miha-stopar self-requested a review February 22, 2024 17:19
@ed255 ed255 added this pull request to the merge queue Feb 22, 2024
Merged via the queue into main with commit 8d230b3 Feb 22, 2024
15 checks passed
@ed255 ed255 deleted the feature/circuit-stats branch February 22, 2024 18:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@han0110 han0110 han0110 approved these changes

@rrtoledo rrtoledo rrtoledo left review comments

@ChihChengLiang ChihChengLiang ChihChengLiang left review comments

@KimiWu123 KimiWu123 KimiWu123 approved these changes

@miha-stopar miha-stopar miha-stopar approved these changes

Assignees
No one assigned
Labels
crate-gadgets Issues related to the gadgets workspace member crate-integration-tests Issues related to the integration-tests workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Report halo2 stats of each sub circuit
6 participants
@ed255 @miha-stopar @ChihChengLiang @rrtoledo @KimiWu123 @han0110