Refactor Supernova benchmarks and config for CI #253
Conversation
samuelburnham
force-pushed
the
bench-refactor
branch
from
af3f562 to
51ea08a
Compare
samuelburnham
force-pushed
the
bench-refactor
branch
from
51ea08a to
6c6fe76
Compare
benches/justfile
Outdated
| gpu-bench-ci +benches: | ||
| #!/bin/sh | ||
| for bench in {{benches}}; do | ||
| cargo criterion --bench $bench --features "cuda" --message-format=json > "$bench-{{commit}}".json | ||
| cargo criterion --bench $bench --features "cuda,asm" --message-format=json > "$bench-{{commit}}".json |
There was a problem hiding this comment.
Are we sure all uses of this justfile are on x86_64? If not, the ASM feature will cause a panic here.
We may want to add at least a code comment, if not running arch beforehand and doing the feature selection in function.
benches/common/supernova/bench.rs
Outdated
| // we vary the number of constraints in the step circuit | ||
| for &num_cons_in_augmented_circuit in num_cons().iter() { | ||
| // number of constraints in the step circuit | ||
| let num_cons = num_cons_in_augmented_circuit - NUM_CONS_VERIFIER_CIRCUIT_PRIMARY; |
There was a problem hiding this comment.
Yeah, so this is the problem: in bench mode, that substraction has wrapping semantics, and if a missed update of the cons numbers makes it wrap around, the benches will silently start eating up the bench machine's memory. We should do checked_sub(..).expect("constraint numbers out of date!!") here
benches/common/supernova/mod.rs
Outdated
| // This should match the value in test_supernova_recursive_circuit_pasta | ||
| // TODO: This should also be a table matching the num_augmented_circuits in the below | ||
| pub const NUM_CONS_VERIFIER_CIRCUIT_PRIMARY: usize = 9844; | ||
| pub const NUM_SAMPLES: usize = 10; |
There was a problem hiding this comment.
Not-so-fun part: this number is meant to be meaningfully different for the nova and supernova use cases. We may want to document that.
This number is also a footgun waiting to happen, see my note on let num_cons = ...
…-lang#253) * add rayon for prove_inner * version
* optimize ppsnark (lurk-lang#252) * optimize; check claims about Az,Bz,Cz * add rayon * Use rayon for computing evaluations in prove_helper in parallel (lurk-lang#253) * add rayon for prove_inner * version --------- Co-authored-by: Srinath Setty <[email protected]>
This PR refactors the Supernova benchmarks to reduce duplicated code and enables configuration of the constraint numbers used in the benchmarks via the
ARECIBO_NUM_CONSenv var.benches/common/supernova. Thebench_snark_internal_with_arityfunction is a bit hacky, as theS1, S2trait bounds are required but unused by the recursive SNARK. There is surely a better trait solution but this seemed simplest for now.supernova-cibenchmark to run theRecursiveSNARKSuperNova-2circuitandCompressedSNARKSuperNova-Commitments-2circuitwith 16384/1038732 constraints to get a good idea of perf changes on each PR, and updates CI accordingly.quote!-ing a vector of strings into a list of tokens and passing that to thecriterion_group!targets, but it expected apathspecifier instead of anexprso this was unsuccessful. Other than this I couldn't think of a way to configure which targets to run besides feature configuration or the implemented solution of creating a new benchmark.