Lagrange basis aurora speedup2#30
Lagrange basis aurora speedup2#30pwang00 wants to merge 13 commits intoscipr-lab:lagrange_basis_aurora_speedup2from
Conversation
…o .shift() (scipr-lab#24) * Address issue 18: consolidate variable / method names for subspaces and cosets into .shift() * Fix signature in field_subset.tcc * Remove & from shift method signature * Remove & from shift method signature
ValarDragon
left a comment
ValarDragon
left a comment
There was a problem hiding this comment.
Nice work! Couple of changes suggested!
ValarDragon
changed the base branch from
lagrange_basis_aurora_speedup2
to
master
ValarDragon
changed the base branch from
master
to
lagrange_basis_aurora_speedup2
| @@ -94,24 +94,28 @@ std::shared_ptr<std::vector<FieldT>> multi_lincheck_virtual_oracle<FieldT>::eval | |||
| * [TODO: cite Succinct Aurora] instead of powers of alpha. */ | |||
There was a problem hiding this comment.
Can you change that citation to be [BCGGRS19]?
There was a problem hiding this comment.
err, no the original citation was right. I meant can you change the TODO haha
There was a problem hiding this comment.
err, no the original citation was right. I meant can you change the TODO haha
Oh my bad, yeah will be fixed in upcoming commit.
| constraint_domain_vanishing_polynomial_inverses = batch_inverse(this->constraint_domain_vanishing_polynomial_ | ||
| .evaluations_over_field_subset(this->codeword_domain_)); | ||
|
|
||
| for (int i = 0; i < variable_domain_vanishing_polynomial_evaluations.size(); i++) |
There was a problem hiding this comment.
Minor note, can you put the following line of code in a { } block? Consistency with this sort of thing makes it easier to review the final code base. Hope to integrate linting soon, so there will be an automatic check for this sort of thing.
|
|
||
|
|
||
| /* If |variable_domain| > |constraint_domain|, we multiply the Lagrange sampled | ||
| polynomial by Z_{variable_domain}*Z_{constraint_domain}^-1 */ |
There was a problem hiding this comment.
Lagrange sampled polynomial -> Lagrange sampled polynomial (p_alpha_prime)
| FieldT p_alpha_prime_X = this->p_alpha_.evaluation_at_point(evaluation_point);; | ||
| // Depending on the cardinalities of the constraint / variable domain | ||
| // multiplies the constraint domain vanishing polynomial Z_C = (Z_C(a) - Z_C(X)) / (a - X) with Z_V / Z_C, where | ||
| // Z_V is the variable domain vanishing polynomial. Since polynomials typically don't have inverses, |
There was a problem hiding this comment.
Can you make this comment the same as the one in evaluated_contents
2 participants
Addresses #28