-
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 Boundary Constraints #27
base: master
Are you sure you want to change the base?
Conversation
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.
The only delta compared to the other PR is air/src/boundary.rs
right?
let pos = constraints | ||
.iter() | ||
.position(|constraint: &ConstraintData<F>| constraint.point_x == x); |
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.
Might be fine since the number of boundary constraints is probably small, but this has O(n^2) complexity.
Maybe use a Map : point_x -> point_y
Then collect into ConstraintData at the end.
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 is the use case of "overwriting" a previous boundary constraint with a different point_y? Should we just error instead?
let mut mask = Vec::with_capacity(n_columns); | ||
for i in 0..n_columns { | ||
mask.push((0, i)); | ||
} |
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.
let mask = (0..n_columns).map(|i| (0, i)).collect()
fn constraints_eval( | ||
&self, | ||
neighbors: &[F], | ||
_periodic_columns: &[F], | ||
random_coefficients: &[F], | ||
point: &F, | ||
_shifts: &[F], | ||
_precomp_domains: &[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.
How do we compose this with other types of constraints?
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 haven't reviewed the Stone code where boundary constraints are composed with other types of constraints, so I can't answer this yet.
The code in the following commit is the delta |
This PR implements boundary constraints which are part of the following library.
https://github.com/starkware-libs/stone-prover/tree/main/src/starkware/air
This PR has dependency on #26.