Ns/bench tfhe zk pok #1771
Ns/bench tfhe zk pok #1771
Conversation
nsarlin-zama
force-pushed
the
ns/bench_tfhe_zk_pok
branch
from
3033067 to
358f1a4
Compare
There was a problem hiding this comment.
I think something is not right with the k * 64, seeding feels unnecessary as we have randomness in real world performance/rng
We must find a way to avoid the duplication for the pure in house benchmark utils stuff and the zk-sepcific parameters setup this won't be maintainable long term
| # Placeholder workflow file allowing running it without having to merge to main first | ||
| name: Placeholder Workflow | ||
| # Run benchmarks of the tfhe-zk-pok crate on an instance and return parsed results to Slab CI bot. | ||
| name: tfhe-zk-pok benchmarks |
There was a problem hiding this comment.
to be reverted once you are done testing
tfhe-zk-pok/benches/pke_prove.rs
Outdated
| (PKEV2_TEST_PARAMS, "PKEV2_TEST_PARAMS"), | ||
| ] { | ||
| let (public_param, public_commit, private_commit, metadata) = init_params_v1(params); | ||
| let bits = params.k * 64; |
| @@ -0,0 +1,459 @@ | |||
| #![allow(non_snake_case)] | |||
There was a problem hiding this comment.
looks to be mostly a copy paste ?
There was a problem hiding this comment.
I need the types in tfhe/benches/utilities to be able to create the json file with the params but I can't use it directly because I am not in the tfhe crate. This is mostly a copy paste with only the needed bits and tfhe internal types converted to builtin rust types.
I also integrated things from tfhe-zk-pok/proof/mod.rs test module because benches are not run with #[cfg(test)]
There was a problem hiding this comment.
the utils can be marked with a cfg = "bench" I believe
I think we have something like that in TFHE-rs
as for the utils some work needs to be done with David to make the copied code into a small util crate, we can't copy things when we "just" need to bench something
There was a problem hiding this comment.
You think about a crate like tfhe-utils that would contains CryptoParametersRecord and write_to_json() function ?
There was a problem hiding this comment.
potentially, we can't duplicate those everywhere
There was a problem hiding this comment.
The problem is that the current bench utilities make heavy use of tfhe types so it cannot be used without it, it would need a rework that's out of the scope of this PR I think. Since tfhe itself depends on tfhe-zk-pok it should be written in a way that prevent circular dependencies.
I don't think there is a cfg(bench) and the code is already only compiled with benches, the problem is that I can't use code from cfg(test) from there.
There was a problem hiding this comment.
see tfhe/src/core_crypto/commons/ciphertext_modulus.rs:23 fairly sure there is something for benchmarks
There was a problem hiding this comment.
Are you sure that this works ? I don't find any doc about it, it does not seem to have any effect on my code and cargo warns about it. Maybe there is some special trick in the tfhe crate to make it work ?
|
manual workflow run: https://github.com/zama-ai/tfhe-rs/actions/runs/11800194113/job/32870653319 |
| @@ -0,0 +1,459 @@ | |||
| #![allow(non_snake_case)] | |||
There was a problem hiding this comment.
You think about a crate like tfhe-utils that would contains CryptoParametersRecord and write_to_json() function ?
nsarlin-zama
force-pushed
the
ns/bench_tfhe_zk_pok
branch
5 times, most recently
from
ecaa1a3 to
fc85bbc
Compare
nsarlin-zama
force-pushed
the
ns/bench_tfhe_zk_pok
branch
from
fc85bbc to
f900480
Compare
nsarlin-zama
force-pushed
the
ns/bench_tfhe_zk_pok
branch
from
f900480 to
4938df4
Compare
PR content/description
Add benches to the tfhe-zk-pok crates to be able to compare timings between zk v1 and v2