-
Notifications
You must be signed in to change notification settings - Fork 20
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(example): cli #275
feat(example): cli #275
Conversation
**Motivation** Now we have a default path for time-profling & mem-profiling, but it is not very convenient to vary the runtime parameters by hand. That's why I put all runtime parameters in a separate cli example. This allows us to do time-profiling & mem-profiling on different parameters directly on the command line **Overview** Fairly simple code that parses parameters with clap and runs circuit. Covers all our examples Once mem-profiling is in main, I'll add its description to the README along with this example
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.
IIUC, this cli example is to profile one specific combination of parameters (i.e. specify one table size, one commitment key size etc) while the bench folder will profile different combinations of parameters(i.e. specify various table sizes and key sizes combinations)?
bincode = "1.3" | ||
clap = { version = "4.5.4", features = ["derive"] } | ||
criterion = "0.5.1" | ||
halo2_gadgets = { git = "https://github.com/snarkify/halo2", branch = "snarkify/dev", features = ["unstable"] } |
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.
can you remind me where is halo2_gadgets used?
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 didn't add it as a dependency now, it's just that the add dependency utility lexicographically sorted out all the crates.
And so it is probably not needed, because only BLOCK_SIZE
is imported from it. We should add a cargo-udeps utility to our pipeline to check dependencies for necessity automatically
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.
yeah, I saw it is reordered, but I didn't find where it used. Probably just outdated.
Memory profiling with dhat works on the whole process, so you have to run one process many times and there is no convenient benchmark option. If it were not for this limitation, we would just do the same as in the benchs folder |
Motivation
Now we have a default path for time-profling & mem-profiling, but it is not very convenient to vary the runtime parameters by hand. That's why I put all runtime parameters in a separate cli example. This allows us to do time-profiling & mem-profiling on different parameters directly on the command line
Part of #272
Overview
Fairly simple code that parses parameters with clap and runs circuit. Covers all our examples
Once mem-profiling is in main, I'll add its description to the README along with this example