refactor(honk): template honk composers by flavor #488
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maramihali
force-pushed
the
mm/honk-flavor-template
branch
2 times, most recently
from
b2a3972 to
910669c
Compare
maramihali
force-pushed
the
mm/honk-flavor-template
branch
2 times, most recently
from
9c084ab to
2877918
Compare
maramihali
force-pushed
the
mm/honk-flavor-template
branch
from
2877918 to
c218260
Compare
maramihali
changed the title
maramihali
changed the title
codygunton
reviewed
Jun 12, 2023
| @@ -1,5 +1,6 @@ | |||
| #include "acir_format.hpp" | |||
| #include "barretenberg/common/log.hpp" | |||
| #include "barretenberg/stdlib/primitives/composers/composers.hpp" | |||
There was a problem hiding this comment.
Why is this suddenly needed?
There was a problem hiding this comment.
Here and elsewhere, we'd rather be more specific than include this one header because it's implicitly including a lot of other stuff: all composers and circuit constructors.
| @@ -957,6 +957,10 @@ typename Curve::Element pippenger_without_endomorphism_basis_points(typename Cur | |||
|
|
|||
| // Explicit instantiation | |||
| // BN254 | |||
| template void generate_pippenger_point_table<curve::BN254>(curve::BN254::AffineElement* points, | |||
There was a problem hiding this comment.
It turned out to not be necessary to explicitly instantiate this function (with or without extern, for either curve).
|
|
||
| namespace proof_system::honk::flavor { | ||
| class StandardGrumpkin { | ||
| // TODO(Mara): At the moment this class is a duplicate of the Standard flavor with a different PCS for testing |
There was a problem hiding this comment.
I think it's probably right to just duplicate this as you have.
| @@ -412,4 +419,23 @@ template typename honk::flavor::Ultra::Polynomial compute_sorted_list_accumulato | |||
| template void add_plookup_memory_records_to_wire_4<honk::flavor::Ultra>( | |||
| std::shared_ptr<typename honk::flavor::Ultra::ProvingKey>& key, typename honk::flavor::Ultra::FF eta); | |||
|
|
|||
| template honk::flavor::UltraGrumpkin::Polynomial compute_permutation_grand_product<honk::flavor::UltraGrumpkin>( | |||
There was a problem hiding this comment.
Ugh, maybe this should go in a (stateless) class so we can greatly simplify explicit instantiation? Not asking you to do that, just complaining out loud.
ludamad
pushed a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jul 22, 2023
…etenberg#488) --------- Co-authored-by: maramihali <[email protected]> Co-authored-by: codygunton <[email protected]> Co-authored-by: Mara Mihali <[email protected]>
ludamad
pushed a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Jul 24, 2023
…etenberg#488) --------- Co-authored-by: maramihali <[email protected]> Co-authored-by: codygunton <[email protected]> Co-authored-by: Mara Mihali <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds composer templating by flavor and bring us closer to enabling Honk on Grumpkin. Additionally, it specs the forward declaration of composers in the
stdliband makes sure thestdlibclasses do not import the composers directly from the proof system folders as this is very prone to linker problemsChecklist:
/markdown/specshave been updated.@briefdescribing the intended functionality.