-
Notifications
You must be signed in to change notification settings - Fork 136
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
feat(field): add mersenne31 prime field #565
Conversation
Codecov Report
@@ Coverage Diff @@
## main #565 +/- ##
==========================================
- Coverage 95.61% 95.58% -0.04%
==========================================
Files 111 113 +2
Lines 19310 19595 +285
==========================================
+ Hits 18464 18730 +266
- Misses 846 865 +19
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@MauroToscano @schouhy Bench results for the Mersenne31 backend in comparison to a U64 montgomery backend |
The benchmark seems wrong, it's based in the old benchmarks of the Math library which are also not good. When using Criterion, the compiler may optimize when constants that are always the same. That's why you see one Pow taking 300 ps, which is highly suspicious. If you blackbox the paramater with "bench.iter(|| x.pow(black_box(*y)));", now the mersenne's backend is faster. Which is nice! If you want to copy the ones of Math, you will have to fix this kind of issues. If you see some difference that big it may be the compiler optimizing something that it shouldn't be optimizing. The ones used here are more solid: https://github.com/lambdaclass/lambdaworks/tree/main/benches. We can improve the ones in math and fix this kind of issues. |
@MauroToscano Appreciate the feedback will get on it! |
Looks good now |
1482330
to
77ba067
Compare
@MauroToscano @schouhy I finished adding the field extensions and hooking into libfuzzer. One note is the lambdaworks quadratic extension differed slightly from the implementation of the inv() operation for the Mersenne31 fields complex extension in plonky3 |
016dc74
to
8f7b415
Compare
Adds Mersenne31 Prime Field
Description
Description of the pull request changes and motivation.
Starts #540
Adds an implementation of the Mersenne31 prime field following the implementation in Plonky 3.
Please delete options that are not relevant.
Checklist