-
Notifications
You must be signed in to change notification settings - Fork 138
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 benchmark templates and sample benches for IPA #133
Conversation
Co-authored-by: Antonio Mejías Gil <[email protected]>
benches/benches.rs
Outdated
@@ -0,0 +1,57 @@ | |||
// #![cfg(feature = "benches")] |
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.
// #![cfg(feature = "benches")] |
Cargo.toml
Outdated
criterion = { version = "0.5", default-features = false, optional = true } | ||
rand_chacha = { version = "0.3.0", default-features = false, optional = true } |
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.
hm we don't want criterion
as a normal dependency, as it has too many dependencies. criterion
should be a dev-dependency.
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.
Also, rand_chacha
is already a dev-dependency.
src/bench_templates/mod.rs
Outdated
@@ -0,0 +1,241 @@ | |||
use ark_crypto_primitives::sponge::{ |
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 make this a file in benches
, or extract it into a separate crate (ala ark-algebra-bench-templates
)
Thanks @Pratyush, I've refactored the bench templates into its own crate as suggested. This required creating a workspace, so there's a bunch of files moved around, but ultimately the actual changes are few and much more elegant. Adding benches for a new scheme will also hopefully be simpler. |
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.
Thanks for the change! The only thing that I think we should undo is the pcs
renaming; it's fine to have the repo be called poly-commit and the folder within also called poly-commit.
* Add benchmark templates and sample benches for IPA Co-authored-by: Antonio Mejías Gil <[email protected]> * adapt max poly degree to 20 * Move all PC Schemes to one module; bench-templates as a separate module * inline one of the type aliases * remove commented out comment * Add benchmark instructions to the README * make bench-templates non publishable * fix yaml file: remove newline * rename the inner dir to `poly-commit` * rename usages of `pcs` to `poly-commit` --------- Co-authored-by: Antonio Mejías Gil <[email protected]>
Description
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
Pending
section inCHANGELOG.md
Files changed
in the Github PR explorer