-
Notifications
You must be signed in to change notification settings - Fork 141
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
Baby bear extension #942
base: main
Are you sure you want to change the base?
Baby bear extension #942
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #942 +/- ##
==========================================
+ Coverage 71.04% 71.12% +0.07%
==========================================
Files 153 153
Lines 32770 33101 +331
==========================================
+ Hits 23283 23542 +259
- Misses 9487 9559 +72 ☔ View full report in Codecov by Sentry. |
<Degree4BabyBearExtensionField as IsField>::add(a, a) | ||
} | ||
|
||
fn pow<T>(a: &Self::BaseType, mut exponent: T) -> Self::BaseType |
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.
This can be written simpler. There are some repetitions of code
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.
Solved here
[ | ||
FieldElement::zero(), | ||
FieldElement::zero(), | ||
FieldElement::zero(), | ||
FieldElement::zero(), | ||
] |
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.
[ | |
FieldElement::zero(), | |
FieldElement::zero(), | |
FieldElement::zero(), | |
FieldElement::zero(), | |
] | |
Self::BaseType::default() |
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.
Solved here
impl ByteConversion for [FieldElement<Babybear31PrimeField>; 4] { | ||
#[cfg(feature = "alloc")] | ||
fn to_bytes_be(&self) -> alloc::vec::Vec<u8> { | ||
unimplemented!() | ||
} | ||
|
||
#[cfg(feature = "alloc")] | ||
fn to_bytes_le(&self) -> alloc::vec::Vec<u8> { | ||
unimplemented!() | ||
} | ||
|
||
fn from_bytes_be(_bytes: &[u8]) -> Result<Self, crate::errors::ByteConversionError> | ||
where | ||
Self: Sized, | ||
{ | ||
unimplemented!() | ||
} | ||
|
||
fn from_bytes_le(_bytes: &[u8]) -> Result<Self, crate::errors::ByteConversionError> | ||
where | ||
Self: Sized, | ||
{ | ||
unimplemented!() | ||
} | ||
} |
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 think it's better to implement this methods before merging the PR.
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.
Implemented here
prop_assert_eq!(fft_eval, naive_eval); | ||
} | ||
|
||
// #[cfg(not(any(feature = "metal"),not(feature = "cuda")))] |
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.
if this line will be commented out, it's better to remove it
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.
Removed in here
Quartic Baby Bear Extension
Description
Add the field extension of Baby Bear of degree 4 using the irreducible polynomial x^4 + 11.
Type of change
Please delete options that are not relevant.