Deterministic tests - Githubissues

privacy-scaling-explorations / halo2

https://privacy-scaling-explorations.github.io/halo2/

Other

207 stars 129 forks source link

Deterministic tests #349

Closed adria0 closed 4 months ago

adria0 commented 4 months ago

New crate halo2_debug with some utilities, it can be reused for https://github.com/privacy-scaling-explorations/halo2/issues/343
Remove integrated dhat profiling, it is activated in cargo test in M3 as also in coverage, creating crashes (since it's already activated). Also remove the associated heap-profiling feature (that is activated with --all-features)
Add new feature vector-tests, note that the main idea here is that this feature is activated via --all-features so the vector tests are specific to --all-features

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 53.33333% with 7 lines in your changes missing coverage. Please review.

Project coverage is 82.06%. Comparing base (32599e8) to head (3d7cca4).

Files	Patch %	Lines
halo2_debug/src/lib.rs	53.33%	7 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #349 +/- ## ========================================== - Coverage 82.09% 82.06% -0.03% ========================================== Files 83 84 +1 Lines 17228 17243 +15 ========================================== + Hits 14143 14151 +8 - Misses 3085 3092 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

adria0 commented 4 months ago

I am not sure about the change in the PCS. Particularly, the addition of a rng parameter in new. Seems like IPA doesn't use it and KZG keeps using OsRng.

I wonder if there is a cleaner way to get deterministic params for the tests. What do you think about leaving new as is, and adding a function that generates some fixed kzg parameters for testing?

Sure, a good option, there's the ::setup there

adria0 commented 4 months ago

@ed255 recheck again plz. Happened that with multithreading we get different results with OsX and Linux, so I limited the number of threads and fixed the CI.