-
Notifications
You must be signed in to change notification settings - Fork 112
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
Prover constructs sumcheck #61
Conversation
aa1307a
to
771845c
Compare
02fe345
to
b0f7570
Compare
c58872c
to
4aac909
Compare
} | ||
}; | ||
|
||
explicit Multivariates(const std::shared_ptr<waffle::proving_key>& proving_key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be updated to handle shifted polynomials in follow-on
@@ -70,7 +74,18 @@ TEST(Sumcheck, Prover) | |||
q_c, sigma_1, sigma_2, sigma_3, id_1, id_2, id_3, lagrange_1 | |||
}; | |||
|
|||
auto transcript = Transcript(transcript::Manifest()); | |||
std::vector<transcript::Manifest::RoundManifest> manifest_rounds; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-on: Stuff like this and mock_prover_contributions_to_transcript
could be replaced with the real manifest and the mock_inputs_prior_to_challenge()
fctn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think, more generally, we need to decide on a policy about when we use real things vs mocked things. @arijitdutta67 maybe you can keep this in mind as you work on the sumcheck tests and implement @ledwards2225's suggestion if you think it makes sense to do so?
PolynomialDescriptor("W_2", "w_2_lagrange", false, false, WITNESS, W_2), // | ||
PolynomialDescriptor("W_3", "w_3_lagrange", false, false, WITNESS, W_3), // | ||
PolynomialDescriptor("Z_PERM", "z_perm", true, true, WITNESS, Z), // | ||
// PolynomialDescriptor("Z_PERM_SHIFT", "z_perm_shift", true, true, WITNESS, Z_LOOKUP), // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be reinstated in follow-on
@@ -11,7 +11,7 @@ struct StandardArithmetization { | |||
W_R, | |||
W_O, | |||
Z_PERM, | |||
Z_PERM_SHIFT, | |||
Z_PERM_SHIFT, // TODO(Cody): Hid ethis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be addressed in follow-on
@@ -36,7 +36,8 @@ struct StandardHonk { | |||
public: | |||
using Arithmetization = proving_system::StandardArithmetization; | |||
using MULTIVARIATE = Arithmetization::POLYNOMIAL; | |||
static constexpr size_t STANDARD_UNROLLED_MANIFEST_SIZE = 12; // cf waffle::STANDARD_UNROLLED_MANIFEST_SIZE | |||
// TODO(Cody): Where to specify? is this polynomial manifest size? | |||
static constexpr size_t STANDARD_HONK_MANIFEST_SIZE = 16; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this just be deleted? Not currently used anywhere. Or should the value be specified here then used by the polynomial manifest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it in only as a comment, as a suggestion that perhaps this data should be specified as part of the flavor.
}, | ||
/* challenge_name = */ "rho", | ||
/* num_challenges_in = */ STANDARD_UNROLLED_MANIFEST_SIZE - 1, /* TODO(Cody): this is bad. */ | ||
/* num_challenges_in = */ 11, /* TODO(Cody): magic number! Where should this be specified? */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be addressed in follow-on
@@ -233,6 +234,9 @@ std::shared_ptr<waffle::proving_key> ComposerHelper<CircuitConstructor>::compute | |||
|
|||
compute_first_and_last_lagrange_polynomials(circuit_proving_key.get()); | |||
|
|||
// TODO(Cody): this is a workaround |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be addressed in follow-on
* Hack poly mfst to get multivariates from pk. * d is not a template param * Prover executes sumcheck. * Address some comments from Luke.
* Hack poly mfst to get multivariates from pk. * d is not a template param * Prover executes sumcheck. * Address some comments from Luke.
* Hack poly mfst to get multivariates from pk. * d is not a template param * Prover executes sumcheck. * Address some comments from Luke.
* Hack poly mfst to get multivariates from pk. * d is not a template param * Prover executes sumcheck. * Address some comments from Luke.
This PR makes the Honk Prover class construct a sumcheck object and execute this. Some of the higher-level changes this necessitates.
multivariate_d
, but at least for now I make this size a runtime parameter. This necessitates replacing some uses ofstd::array
withstd::vector
.Multivariates
object from a proving key (needed for prover to create the object) and from a transcript (Sumcheck class in verifier mode had previously not constructed multivariates).I regret leaving the insertion of the sumcheck evaluations in a rough form in the prover. This is done for expediency.