feat(zk): add GPU ZK long-run tests and CI workflow#3453
feat(zk): add GPU ZK long-run tests and CI workflow#3453
Conversation
778b322 to
5f8fc4e
Compare
There was a problem hiding this comment.
I see this output:
running 1 test
test gpu::tests::prove_verify_stress::test_pke_v2_gpu_cpu_equivalence_long_run ... test_pke_v2_gpu_cpu_equivalence_long_run: base_seed=0x7ca2d9fd80287902, rounds=3
round 0/3: seed=0x61bbcfdbee87953a
round 1/3: seed=0x2a5b618e3fcba7e0
round 2/3: seed=0xce7587440ec88f6d
ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 54 filtered out; finished in 505.59s
Doc-tests tfhe_zk_pok
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Is this ok ? only 3 rounds of one test are run ?
can't we group this with some other workflow ? we're adding more CI costs just for 6 minutes of tests
| thread_rng().gen() | ||
| } | ||
|
|
||
| /// Run one full GPU-vs-CPU equivalence sweep for a given seed. |
There was a problem hiding this comment.
this comment isn't very clear. the values in the comment may change. the comment should explain what the function does not give the constants used.
| /// they agree. | ||
| #[test] | ||
| fn test_pke_v2_gpu_cpu_equivalence() { | ||
| let seed: u64 = thread_rng().gen(); |
There was a problem hiding this comment.
why don't we allow to use the user supplied seed here? why print the seed if we can't reproduce with it ?
There was a problem hiding this comment.
Which user supplied seed?
The seed print was once requested by @nsarlin-zama. In case some test fails we can easily reproduce the execution path.
There was a problem hiding this comment.
if we print the seed we may want to re-use it easily through "TFHE_RS_LONGRUN_TESTS_SEED", why don't we use that here ?
|
|
||
| /// Multi-seed long-run variant of the GPU/CPU equivalence test. | ||
| /// | ||
| /// Repeats the full sweep (3 CRS variants x 32 invalid-witness flags |
There was a problem hiding this comment.
the comment is too verbose. no need to explain environment variables again
5f8fc4e to
e0f2e3f
Compare
|
@andrei-stoian-zama I removed most of the comments. Check again, please. |
|
@andrei-stoian-zama I don't know why you got only 3 runs. Here I see 20. I'm not familiar with the other long run tests. Which one do you think would fit this test? |
| on: | ||
| workflow_dispatch: | ||
| schedule: | ||
| - cron: "0 20 * * *" |
There was a problem hiding this comment.
let's only run it on Mondays (integer long run will run the same way)
| workflow_dispatch: | ||
| schedule: | ||
| - cron: "0 20 * * *" | ||
| pull_request: |
There was a problem hiding this comment.
please use an appropriate "should_run" filter - only changes to ZK trigger it
| }); | ||
| } | ||
|
|
||
| let is_minimal = |
There was a problem hiding this comment.
if it's set and not "TRUE" then please assert. otherwise we'll make mistakes with the syntax
| /// they agree. | ||
| #[test] | ||
| fn test_pke_v2_gpu_cpu_equivalence() { | ||
| let seed: u64 = thread_rng().gen(); |
There was a problem hiding this comment.
if we print the seed we may want to re-use it easily through "TFHE_RS_LONGRUN_TESTS_SEED", why don't we use that here ?
closes: https://github.com/zama-ai/tfhe-rs-internal/issues/1372
PR content/description
Check-list: