-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add ntt for babybear and goldilocks #144
Conversation
impl From<usize> for Goldilocks { | ||
#[inline] | ||
fn from(value: usize) -> Self { | ||
let modulus = Goldilocks::MODULUS_VALUE as usize; | ||
if value < modulus { | ||
Self(value as u64) | ||
} else { | ||
Self((value % modulus) as u64) | ||
} | ||
} | ||
} |
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 here value - modulus
is enough for case value >= modulus
?
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.
Yes, it is sufficient. I did not mean to optimize this.
|
||
use crate::{transformation::prime32::ConcreteTable, Field, NTTField}; | ||
|
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.
Use ConcreteTable
make this implementation is not correctly when feature concrete-ntt
is disabled.
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.
We do not have a non-concrete version yet. That is why I do not use a feature here, which means we have to always use concrete for these two fields.
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.
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.
We can disable the default feature to disable concrete-ntt
.
fn try_primitive_root(degree: Self::Degree) -> Result<Self, crate::AlgebraError> { | ||
let modulus_sub_one = BabyBear::MODULUS_VALUE - 1; | ||
let quotient = modulus_sub_one / degree; | ||
if modulus_sub_one != quotient * degree { | ||
return Err(crate::AlgebraError::NoPrimitiveRoot { | ||
degree: degree.to_string(), | ||
modulus: BabyBear::MODULUS_VALUE.to_string(), | ||
}); | ||
} | ||
|
||
let mut rng = thread_rng(); | ||
let distr = distributions::Uniform::new_inclusive(2, modulus_sub_one); | ||
|
||
let mut w = Self::zero(); | ||
|
||
if (0..100).any(|_| { | ||
w = pow( | ||
Self::new(rand::Rng::sample(&mut rng, distr)), | ||
quotient as usize, | ||
); | ||
Self::is_primitive_root(w, degree) | ||
}) { | ||
Ok(w) | ||
} else { | ||
Err(crate::AlgebraError::NoPrimitiveRoot { | ||
degree: degree.to_string(), | ||
modulus: BabyBear::MODULUS_VALUE.to_string(), | ||
}) | ||
} | ||
} |
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 function is not necessary now. We just use unreachable!()
in it.
fn try_minimal_primitive_root(degree: Self::Degree) -> Result<Self, crate::AlgebraError> { | ||
let mut root = Self::try_primitive_root(degree)?; | ||
|
||
let generator_sq = (root * root).to_root(); | ||
let mut current_generator = root; | ||
|
||
for _ in 0..degree { | ||
if current_generator < root { | ||
root = current_generator; | ||
} | ||
current_generator.mul_root_assign(generator_sq); | ||
} | ||
|
||
Ok(root) | ||
} |
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 function is not necessary now. We just use unreachable!()
in 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.
I think it is used in zk, right?
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.
No, if zk use it, the wrong result will be returned.
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.
It's not the right way to get the root for user. Concrete generate root with a different way.
fn try_primitive_root(degree: Self::Degree) -> Result<Self, crate::AlgebraError> { | ||
let modulus_sub_one = Goldilocks::MODULUS_VALUE - 1; | ||
let quotient = modulus_sub_one / degree; | ||
if modulus_sub_one != quotient * degree { | ||
return Err(crate::AlgebraError::NoPrimitiveRoot { | ||
degree: degree.to_string(), | ||
modulus: Goldilocks::MODULUS_VALUE.to_string(), | ||
}); | ||
} | ||
|
||
let mut rng = thread_rng(); | ||
let distr = distributions::Uniform::new_inclusive(2, modulus_sub_one); | ||
|
||
let mut w = Self::zero(); | ||
|
||
if (0..100).any(|_| { | ||
w = pow( | ||
Self::new(rand::Rng::sample(&mut rng, distr)), | ||
quotient as usize, | ||
); | ||
Self::is_primitive_root(w, degree) | ||
}) { | ||
Ok(w) | ||
} else { | ||
Err(crate::AlgebraError::NoPrimitiveRoot { | ||
degree: degree.to_string(), | ||
modulus: Goldilocks::MODULUS_VALUE.to_string(), | ||
}) | ||
} | ||
} |
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 function is not necessary now. We just use unreachable!()
in it.
fn try_minimal_primitive_root(degree: Self::Degree) -> Result<Self, crate::AlgebraError> { | ||
let mut root = Self::try_primitive_root(degree)?; | ||
|
||
let generator_sq = (root * root).to_root(); | ||
let mut current_generator = root; | ||
|
||
for _ in 0..degree { | ||
if current_generator < root { | ||
root = current_generator; | ||
} | ||
current_generator.mul_root_assign(generator_sq); | ||
} | ||
|
||
Ok(root) | ||
} |
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 function is not necessary now. We just use unreachable!()
in it.
|
||
use crate::{transformation::prime64::ConcreteTable, Field, NTTField}; | ||
|
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.
We need a way to perform when concrete-ntt
is not enable
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 approach is not exactly what I had envisioned, and it seems less than ideal. Nevertheless, it will suffice for the time being. We can always refine it later on.
The impl. of ntt uses concrete-ntt.