-
Notifications
You must be signed in to change notification settings - Fork 2
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
Implement Composition Polynomial #26
base: master
Are you sure you want to change the base?
Conversation
composition_polynomial/src/air.rs
Outdated
#[derive(Clone)] | ||
pub struct DummyAir<F: PrimeField> { | ||
pub trace_length: usize, | ||
pub n_constraints: usize, |
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.
Is this different from self.constraints.len()
?
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.
its the same, removed n_constraints
.
composition_polynomial/src/air.rs
Outdated
pub n_interaction_elements: usize, | ||
} | ||
|
||
type ConstraintFunction<F> = Arc<dyn Fn(&[F], &[F], &[F], &F, &[F], &[F]) -> F>; |
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.
Consider introducing a struct for the argument: that way they are named and it is easy to see what a ConstraintFunction
takes e.g.
struct ConstraintEval {
neighbors: &[F]
periodic_columns: &[F]
random_coefficients: $[F]
point: &[F]
gen_powers: &[F]
precomp_domains: &[F]
}
composition_polynomial/src/air.rs
Outdated
|
||
let mut res = F::ZERO; | ||
for constraint in self.constraints.iter() { | ||
res += constraint( |
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 thought the random_coefficients
where used to combine each constraint? Does each constraint need to take the random coefficients or can we compute the combination here?
None | ||
} | ||
|
||
fn constraints_eval( |
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.
Could this just be a method on CompositionPolynomial<F>
?
e.g. it takes random_coefficients
which is really what defines a CompositionPolynomial
from an AIR. So if we move it there we would not need to take e.g. random_coefficients
vec![point_powers[1] - F::ONE] | ||
} | ||
|
||
fn create_composition_polynomial( |
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.
Could this take ownership of the Air?
i.e.
fn into_composition_polynomial(
self,
trace_generator,
random_coefficients
) -> CompositionPolynomial<F>
That way we avoid the Box and Clone?
let offset = start_point.pow([self.n_copies as u64]); | ||
let n_values = self.lde_manager.base.size as usize; | ||
|
||
assert!( |
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.
nit: assert_eq
} | ||
|
||
impl<F: PrimeField> CosetEvaluation<F> { | ||
pub fn new(values: Vec<F>) -> Self { |
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.
assert!(values.is_power_of_two())
assert!(coset_size % period_in_trace == 0); | ||
let n_copies = coset_size / period_in_trace; | ||
|
||
let offset_compensation = offset.pow([n_copies as u64]).inverse().unwrap(); |
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.
What's the difference between start_point
(as used in get_coset
) and offset
? Both should be the first evaluation point of the first entry of the first copy of the periodic table?
coset_size: usize, | ||
column_step: usize, | ||
) -> Self { | ||
let period_in_trace = values.len() * column_step; |
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.
All these are restricted to powers of two?
} | ||
} | ||
|
||
pub fn eval_at_point(&self, x: F) -> F { |
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.
Nvm: the LDE manager is used to interpolate exactly this polynomial.
Why do we need the lde_manager
?
We have values
and assuming the length of the table divides the group order the polynomial for the column has a succinct description (and can be evaluated efficiently). So can't we just evaluate it directly?
For instance, the polynomial f(X) which evaluates to:
f(1) = y0
f(w) = y1
f(w^2) = y0
f(w^3) = y1
...
i.e. alternating. For a domain of length
Where
So
Here is a Sage script if you want to play around this:
F = GF(2^251 + 17 * 2^192 + 1)
k = 192
g = F.multiplicative_generator()
g = g^(g.multiplicative_order() / 2^k)
order = 2^8
w = g^(g.multiplicative_order() / order)
print(f"order: {order}")
print(g.multiplicative_order())
table = [
1, 2, 1, 2
]
ys = table * (order // len(table))
xs = [w^i for i in range(order)]
R.<X> = PolynomialRing(F)
f = R.lagrange_polynomial(zip(xs, ys))
print(f)
Otherwise, let's hop on a call.
This PR implements the composition polynomial library.
https://github.com/starkware-libs/stone-prover/tree/main/src/starkware/composition_polynomial