Fix circuit size estimation for GoblinUltraHonk #6161
Comments
This was referenced May 2, 2024
lucasxia01
pushed a commit
that referenced
this issue
May 2, 2024
Resolves #6161 Look into the issue for more context on the problem. I moved `add_gates_to_ensure_all_polys_are_non_zero` to be part of `finalize_circuit` for both the goblin and ultra builder. Then upon creating an AcirFormat circuit we now call finalize_circuit. Previously we were having to attach an additional buffer as a hack as we were using `get_total_circuit_size` which correctly estimated a finalized circuit size for plonk, but not for Honk. We will now have these additional gates for UltraPlonk as well, but this is fine as they are not that many gates (~12 gates) and we are removing plonk anyway.
AztecBot
pushed a commit
to AztecProtocol/barretenberg
that referenced
this issue
May 3, 2024
Resolves AztecProtocol/aztec-packages#6161 Look into the issue for more context on the problem. I moved `add_gates_to_ensure_all_polys_are_non_zero` to be part of `finalize_circuit` for both the goblin and ultra builder. Then upon creating an AcirFormat circuit we now call finalize_circuit. Previously we were having to attach an additional buffer as a hack as we were using `get_total_circuit_size` which correctly estimated a finalized circuit size for plonk, but not for Honk. We will now have these additional gates for UltraPlonk as well, but this is fine as they are not that many gates (~12 gates) and we are removing plonk anyway.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In this PR (#6138) we started getting this failure https://app.circleci.com/pipelines/github/AztecProtocol/aztec-packages/37852/workflows/1dab32cb-5733-425a-9793-c49d784ebe42/jobs/1732326 on the
witness_compressionACIR test. We fail on this assertion(degree <= srs->get_monomial_size()). Inside the ProverInstance constructor it looks like we are adding multiple gates that are not being accounted for byget_total_circuit_size()on the ultra circuit builder (which is the method used in the DSL package for estimating the circuit sizes). These extra gates added in the prover instance looks like the most likely culprit.prover_instance.cppthen callscompute_dyadic_sizewhich looks to does similar things toget_total_circuit_size()in the ultra circuit builder except with also including ecc ops as part of the trace's size. We should reimplementget_total_circuit_sizefor the goblin ultra builder to account for any extra reserved gates.The text was updated successfully, but these errors were encountered: