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

fix: correct use of finalize_circuit in bb gates #9066

Closed
wants to merge 1 commit into from

Conversation

maramihali
Copy link
Contributor

@maramihali maramihali commented Oct 8, 2024

In previous PR, I modified bb gates to return the number of gates after the circuit is finalised, not using an estimation with get_num_gates, but without setting ensure_nonzero to true, which means that the dummy gates added to ensure nonzero polynomials were not taken into account. This PR fixes it.

@maramihali maramihali enabled auto-merge (squash) October 8, 2024 09:15
Copy link
Contributor

github-actions bot commented Oct 8, 2024

Changes to circuit sizes

Generated at commit: c4a9d027fd50a7b1e13e46ce202f944390b86b82, compared to commit: 5c77c4f63b2d69c5e28feade2056facafe859e03

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
public_kernel_inner_simulated 0 ➖ 0.00% +15 ❌ +1500.00%
public_kernel_merge_simulated 0 ➖ 0.00% +15 ❌ +1500.00%
public_kernel_tail_simulated 0 ➖ 0.00% +15 ❌ +1500.00%
rollup_base_simulated 0 ➖ 0.00% +15 ❌ +1500.00%
empty_nested 0 ➖ +∞% +17 ❌ +425.00%
empty_nested_simulated 0 ➖ 0.00% +17 ❌ +425.00%
private_kernel_empty_simulated 0 ➖ 0.00% +17 ❌ +425.00%
private_kernel_init_simulated 0 ➖ 0.00% +17 ❌ +425.00%
private_kernel_inner_simulated 0 ➖ 0.00% +17 ❌ +425.00%
private_kernel_reset_simulated 0 ➖ 0.00% +17 ❌ +425.00%
private_kernel_reset_simulated_4_4_4_4_4_4_4_4_1 0 ➖ 0.00% +17 ❌ +425.00%
private_kernel_tail_simulated 0 ➖ 0.00% +17 ❌ +425.00%
private_kernel_tail_to_public_simulated 0 ➖ 0.00% +17 ❌ +425.00%
rollup_block_root_empty 0 ➖ 0.00% +15 ❌ +0.52%
private_kernel_empty 0 ➖ 0.00% +17 ❌ +0.49%
private_kernel_tail 0 ➖ 0.00% +17 ❌ +0.16%
private_kernel_init 0 ➖ 0.00% +17 ❌ +0.06%
parity_base 0 ➖ 0.00% +15 ❌ +0.05%
private_kernel_tail_to_public 0 ➖ 0.00% +17 ❌ +0.04%
private_kernel_inner 0 ➖ 0.00% +17 ❌ +0.03%
private_kernel_reset_4_4_4_4_4_4_4_4_1 0 ➖ 0.00% +17 ❌ +0.02%
private_kernel_reset 0 ➖ 0.00% +17 ❌ +0.00%
public_kernel_inner 0 ➖ 0.00% +14 ❌ +0.00%
public_kernel_merge 0 ➖ 0.00% +14 ❌ +0.00%
rollup_merge 0 ➖ 0.00% +14 ❌ +0.00%
rollup_root 0 ➖ 0.00% +14 ❌ +0.00%
rollup_block_merge 0 ➖ 0.00% +14 ❌ +0.00%
public_kernel_tail 0 ➖ 0.00% +14 ❌ +0.00%
rollup_block_root 0 ➖ 0.00% +14 ❌ +0.00%
rollup_base 0 ➖ 0.00% +14 ❌ +0.00%
parity_root 0 ➖ 0.00% +14 ❌ +0.00%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
public_kernel_inner_simulated 1 (0) 0.00% 16 (+15) +1500.00%
public_kernel_merge_simulated 1 (0) 0.00% 16 (+15) +1500.00%
public_kernel_tail_simulated 1 (0) 0.00% 16 (+15) +1500.00%
rollup_base_simulated 1 (0) 0.00% 16 (+15) +1500.00%
empty_nested 0 (0) +∞% 21 (+17) +425.00%
empty_nested_simulated 1 (0) 0.00% 21 (+17) +425.00%
private_kernel_empty_simulated 1 (0) 0.00% 21 (+17) +425.00%
private_kernel_init_simulated 1 (0) 0.00% 21 (+17) +425.00%
private_kernel_inner_simulated 1 (0) 0.00% 21 (+17) +425.00%
private_kernel_reset_simulated 1 (0) 0.00% 21 (+17) +425.00%
private_kernel_reset_simulated_4_4_4_4_4_4_4_4_1 1 (0) 0.00% 21 (+17) +425.00%
private_kernel_tail_simulated 1 (0) 0.00% 21 (+17) +425.00%
private_kernel_tail_to_public_simulated 1 (0) 0.00% 21 (+17) +425.00%
rollup_block_root_empty 93 (0) 0.00% 2,890 (+15) +0.52%
private_kernel_empty 670 (0) 0.00% 3,485 (+17) +0.49%
private_kernel_tail 4,745 (0) 0.00% 10,944 (+17) +0.16%
private_kernel_init 24,901 (0) 0.00% 30,224 (+17) +0.06%
parity_base 5,371 (0) 0.00% 32,293 (+15) +0.05%
private_kernel_tail_to_public 29,831 (0) 0.00% 41,841 (+17) +0.04%
private_kernel_inner 43,951 (0) 0.00% 54,904 (+17) +0.03%
private_kernel_reset_4_4_4_4_4_4_4_4_1 34,899 (0) 0.00% 73,633 (+17) +0.02%
private_kernel_reset 91,935 (0) 0.00% 470,150 (+17) +0.00%
public_kernel_inner 268,756 (0) 0.00% 516,575 (+14) +0.00%
public_kernel_merge 53,488 (0) 0.00% 1,103,553 (+14) +0.00%
rollup_merge 3,671 (0) 0.00% 1,896,081 (+14) +0.00%
rollup_root 20,239 (0) 0.00% 1,931,568 (+14) +0.00%
rollup_block_merge 20,255 (0) 0.00% 1,931,584 (+14) +0.00%
public_kernel_tail 259,172 (0) 0.00% 2,270,000 (+14) +0.00%
rollup_block_root 4,154 (0) 0.00% 2,837,134 (+14) +0.00%
rollup_base 671,887 (0) 0.00% 3,525,988 (+14) +0.00%
parity_root 5,399 (0) 0.00% 3,775,125 (+14) +0.00%

@maramihali maramihali closed this Oct 8, 2024
auto-merge was automatically disabled October 8, 2024 16:38

Pull request was closed

@lucasxia01
Copy link
Contributor

Subsumed by #9046

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