You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While creating instance during IVC, if we compare the number of gates before and after finalization, we'll get the following:
So there's obviously some bug
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 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
This was resolved with the introduction of get_num_finalized_gates_inefficient() which simply performs finalization. The issue was in the method for estimating finalized gates without actually doing it which has been removed.
Activity
ledwards2225 commentedon 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 infinalize_circuit()and added to thenum_gatestotal. Note thatget_num_gatesdoes not account for "de-duplications" in nnf gates, whereasfinalize_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 returnsnum_gatesdirectly, 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.fix: add TODO with issue for num_gates bug (#4847)
fix: add TODO with issue for num_gates bug (#4847)
codygunton commentedon May 1, 2024
Perhaps this is resolved by AztecProtocol/aztec-packages#5211
ledwards2225 commentedon 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
ledwards2225 commentedon Feb 26, 2026
This was resolved with the introduction of
get_num_finalized_gates_inefficient()which simply performs finalization. The issue was in the method for estimating finalized gates without actually doing it which has been removed.