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

Gate count jumps after finalization #875

Open
Rumata888 opened this issue Feb 28, 2024 · 3 comments
Open

Gate count jumps after finalization #875

Rumata888 opened this issue Feb 28, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@Rumata888
Copy link
Contributor

Rumata888 commented Feb 28, 2024

While creating instance during IVC, if we compare the number of gates before and after finalization, we'll get the following:
image
So there's obviously some bug

@Rumata888 Rumata888 added the bug Something isn't working label Feb 28, 2024
@ledwards2225
Copy link
Collaborator

ledwards2225 commented Feb 28, 2024

Ended up tracking this back to a discrepancy in the non-native-field gate count produced by get_num_gates() and the actual number of nnf gates that are produced in finalize_circuit() and added to the num_gates total. Note that get_num_gates does not account for "de-duplications" in nnf gates, whereas finalize_circuit() does. So before finalization, get_num_gates() overestimates how many nnf gates we'll need. During finalization we actually add fewer than expected due to duplicates. When we call get_num_gates a second time, it doesn't compute it, it just returns num_gates directly, and thus the total is lower. (I suspect this is showing up for the first time or at least more prominently in these IVC tests because our mock circuits are stupid and are probably performing lots of duplicate operations).

One solution could be to add some kind of deduplication logic to get_num_gates. Another could be to simply add an assert that makes sure you've finalized before checking the number of gates.

@codygunton
Copy link
Collaborator

Perhaps this is resolved by AztecProtocol/aztec-packages#5211

@ledwards2225
Copy link
Collaborator

ledwards2225 commented May 1, 2024

@codygunton I don't think so but I'd have to dig in to be sure. The Pr you linked was an ECCVM optimization but the num_gates issue is purely an Ultra issue, i.e. unrelated to Goblin altogether

@codygunton codygunton mentioned this issue Feb 27, 2025
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants