-
Notifications
You must be signed in to change notification settings - Fork 102
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
Brittleness in translator relation tests #784
Closed
Comments
4 tasks
How do you think we can do this? |
codygunton
added a commit
to AztecProtocol/aztec-packages
that referenced
this issue
Nov 22, 2023
The last PR that splits out and completes work from #3022 We add composer, prover and verifier classes for the Goblin Translator VM. We also further flesh out the full Goblin composer tests. Modulo some missing challenge generation, we have our first Goblin proofs (i.e., something worth measuring)! Spawned issues AztecProtocol/barretenberg#783 AztecProtocol/barretenberg#784 AztecProtocol/barretenberg#785 AztecProtocol/barretenberg#786 AztecProtocol/barretenberg#787
AztecBot
pushed a commit
that referenced
this issue
Nov 23, 2023
The last PR that splits out and completes work from AztecProtocol/aztec-packages#3022 We add composer, prover and verifier classes for the Goblin Translator VM. We also further flesh out the full Goblin composer tests. Modulo some missing challenge generation, we have our first Goblin proofs (i.e., something worth measuring)! Spawned issues #783 #784 #785 #786 #787
For completeness: can with an index variable and stash when e.g. &z_perm == &all_entities[i] then z_perm_index = i, then use that index. This should get better once ProverPolynomials owns its memory |
codygunton
pushed a commit
to AztecProtocol/aztec-packages
that referenced
this issue
Nov 30, 2023
Reduces duplication in Flavor classes by using a macro that combines pointer_view with struct fields (along with a new get_all getter that is perhaps more ergonomic) and subclassing. Note that this is a stepping stone, ideally AztecProtocol/barretenberg#788 is used instead of multiple inheritance Also resolves AztecProtocol/barretenberg#784 - Introduces a RefVector class, like `std::vector<T&>` (which is impossible). It is backed by a pointer, and gives operator[] and iterator semantics on the underlying derefenced pointer. Compare std::vector<std::reference_wrapper<T>>, which would operate more like actual pointers (you change the reference, not the value held by the reference). - Introduces a get_all alternative to pointer_view that uses RefVector, update other types to use RefVector, get rid of the HandleType in flavor entity classes - Removes some flavor base classes that didn't do much after array backing removal. Now we get rid of virtual base class methods here, but I don't think it makes sense to do polymorphism in two ways. We can instead use concepts if we want to constrain this with a static assert - Introduces macros that define flavor classes in flavor_macros.hpp. These remove duplication of pointer view and normal listing. - Use inheritance to remove duplication in the larger AllEntities classes in GoblinUltra and GoblinTranslator - Removes modernize-use-nodiscard - too noisy a linter setting, I doubt we'd hit a bug that it would fix (i.e. if you return error codes it can be useful to not ignore them). --------- Co-authored-by: ludamad <[email protected]>
AztecBot
pushed a commit
that referenced
this issue
Dec 2, 2023
Reduces duplication in Flavor classes by using a macro that combines pointer_view with struct fields (along with a new get_all getter that is perhaps more ergonomic) and subclassing. Note that this is a stepping stone, ideally #788 is used instead of multiple inheritance Also resolves #784 - Introduces a RefVector class, like `std::vector<T&>` (which is impossible). It is backed by a pointer, and gives operator[] and iterator semantics on the underlying derefenced pointer. Compare std::vector<std::reference_wrapper<T>>, which would operate more like actual pointers (you change the reference, not the value held by the reference). - Introduces a get_all alternative to pointer_view that uses RefVector, update other types to use RefVector, get rid of the HandleType in flavor entity classes - Removes some flavor base classes that didn't do much after array backing removal. Now we get rid of virtual base class methods here, but I don't think it makes sense to do polymorphism in two ways. We can instead use concepts if we want to constrain this with a static assert - Introduces macros that define flavor classes in flavor_macros.hpp. These remove duplication of pointer view and normal listing. - Use inheritance to remove duplication in the larger AllEntities classes in GoblinUltra and GoblinTranslator - Removes modernize-use-nodiscard - too noisy a linter setting, I doubt we'd hit a bug that it would fix (i.e. if you return error codes it can be useful to not ignore them). --------- Co-authored-by: ludamad <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Change these to not rely on fixed indexing of prover polynomials.
The text was updated successfully, but these errors were encountered: