chore: Random polynomial is now applied on a per-relation term by the Prover #547
Conversation
…tiations of `ValueAcumulatorTypesHelper`
There was a problem hiding this comment.
Looks good to me (once the build errors are fixed, of course). Presumably you're thinking it will be necessary to have subrelations within a single relation that have different requirements re: linear independence? Maybe this will be the case when employing the log deriv if we maintain the same check for "shiftability". (Seems like there's got to be a better way to do that..). Anyway, would make things a bit nicer if the linear indep label could be applied at the Relation level but if this isn't the case then so be it
| @@ -56,6 +56,7 @@ class Standard { | |||
| using Relations = std::tuple<sumcheck::ArithmeticRelation<FF>, sumcheck::PermutationRelation<FF>>; | |||
|
|
|||
| static constexpr size_t MAX_RELATION_LENGTH = get_max_relation_length<Relations>(); | |||
| static constexpr size_t MAX_RANDOM_RELATION_LENGTH = MAX_RELATION_LENGTH + 1; | |||
| static constexpr size_t NUM_RELATIONS = std::tuple_size<Relations>::value; | |||
|
|
|||
| // Instantiate the BarycentricData needed to extend each Relation Univariate | |||
There was a problem hiding this comment.
I think this should now use MAX_RANDOM_RELATION_LENGTH instead of MAX_RELATION_LENGTH to ensure the correct static arrays in barycentric data are computed at compile time
There was a problem hiding this comment.
instantiate_barycentric_utils now uses MAX_RANDOM_RELATION_LENGTH
| @@ -56,6 +56,7 @@ class Standard { | |||
| using Relations = std::tuple<sumcheck::ArithmeticRelation<FF>, sumcheck::PermutationRelation<FF>>; | |||
|
|
|||
| static constexpr size_t MAX_RELATION_LENGTH = get_max_relation_length<Relations>(); | |||
| static constexpr size_t MAX_RANDOM_RELATION_LENGTH = MAX_RELATION_LENGTH + 1; | |||
There was a problem hiding this comment.
A comment explaining that this is length after multiplying by the pow zeta univariate would be helpful here since MAX_RANDOM_RELATION_LENGTH does not immediately make it clear what this is
There was a problem hiding this comment.
added comment
| using Element = std::remove_reference_t<decltype(element)>; | ||
| // Random poly R(X) = (1-X) + X.zeta_pow | ||
| auto random_poly_edge = Univariate<FF, 2>({ 1, pow_univariate.zeta_pow }); | ||
| BarycentricData<FF, 2, extended_size> barycentric_2_to_max_2 = BarycentricData<FF, 2, extended_size>(); |
There was a problem hiding this comment.
Minor nit: At first I thought barycentric_2_to_max_2 was a typo. I see why you did it now but I think something generic like edge_extender or pow_extender would be fine. The original barycentric_2_to_max is also not great since there are two max floating around now
There was a problem hiding this comment.
fixed, renamed to something more descriptive
renamed `barycentric_2_to_max_2` to `pow_zeta_univariate_extender` attempted fix of compiler errors
kevaundray
changed the title
…col/barretenberg into zw/move-random-poly-to-prover
Description
The random polynomial is now applied on a per-relation term by the Prover (instead of being applied to all relations by the Verifier).
Required if we want relations that sum over the hypercube (i.e. we check the sum of the relation terms at each row are zero, instead of checking the relation term is zero at each row).
This is needed to evaluate logarithmic derivatives.
Checklist:
@briefdescribing the intended functionality.includedirectives have been added.