-
Notifications
You must be signed in to change notification settings - Fork 78
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
Formal polynomials. #835
base: dev
Are you sure you want to change the base?
Formal polynomials. #835
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #835 +/- ##
==========================================
- Coverage 91.91% 91.53% -0.38%
==========================================
Files 90 91 +1
Lines 12375 12629 +254
Branches 12375 12629 +254
==========================================
+ Hits 11374 11560 +186
- Misses 892 953 +61
- Partials 109 116 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
5cea2f8
to
8927469
Compare
300bc79
to
83fe6fb
Compare
8927469
to
bd15d94
Compare
83fe6fb
to
88a00ce
Compare
bd15d94
to
b263a69
Compare
88a00ce
to
3bcbb58
Compare
b263a69
to
7a8621d
Compare
3bcbb58
to
6f1ed4d
Compare
7a8621d
to
82374ff
Compare
6f1ed4d
to
8626b9a
Compare
82374ff
to
095fc4e
Compare
8626b9a
to
eb1520f
Compare
eb1520f
to
6a01e82
Compare
095fc4e
to
6e649fc
Compare
6a01e82
to
5a971e9
Compare
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.
Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @Alon-Ti)
crates/prover/src/constraint_framework/poly.rs
line 12 at r2 (raw file):
/// A monic monomial consists of a list of variables and their exponents. #[derive(Debug, Hash, PartialEq, PartialOrd, Eq, Ord, Clone)]
Nit: is hash needed?
Suggestion:
#[derive(Debug, PartialEq, PartialOrd, Eq, Ord, Clone)]
crates/prover/src/constraint_framework/poly.rs
line 31 at r2 (raw file):
/// A polynomial consists of a list of monomials with coefficients. #[derive(Debug, Hash, PartialEq, PartialOrd, Eq, Ord, Clone)]
Same here.
Suggestion:
#[derive(Debug, PartialEq, PartialOrd, Eq, Ord, Clone)]
crates/prover/src/constraint_framework/poly.rs
line 46 at r2 (raw file):
impl<F: From<BaseField>> From<F> for Polynomial<F> where F: One + Clone,
Suggestion:
impl<F: One + Clone + From<BaseField>> From<F> for Polynomial<F>
crates/prover/src/constraint_framework/poly.rs
line 151 at r2 (raw file):
impl<F: Zero + Clone + From<BaseField>> Zero for Polynomial<F> { fn is_zero(&self) -> bool { self.monomials.is_empty()
Can multiple zero representations not returning true here? i.e. x0^0 - x1^0
Is it a problem?
Code quote:
self.monomials.is_empty()
This change is