Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Revive Fault Proof Part 1 #920

Merged
merged 3 commits into from
Sep 29, 2023
Merged

Revive Fault Proof Part 1 #920

merged 3 commits into from
Sep 29, 2023

Conversation

SchmErik
Copy link
Contributor

This is an attempt to re-enable proof of fault. With cycle-sensitive tests now only running in the docker environment, this change shouldn't impact CI with cycle-related test failures that show different results by architecture.

@SchmErik
Revive Fault Proof Part 1
e68b9e1 
This is an attempt to re-enable proof of fault. With several tests now only
running in the docker environment, this change shouldn't impact CI.
@SchmErik SchmErik enabled auto-merge (squash) September 28, 2023 22:11
@SchmErik SchmErik requested a review from flaub September 28, 2023 22:11
@github-actions
Copy link

Benchmark for Linux-cuda

    <details open>
      <summary>Click to hide benchmark</summary>
      Benchmarks have changed between the two branches, unable to diff.
    </details>

Benchmark for Linux-default

    <details open>
      <summary>Click to hide benchmark</summary>
      Benchmarks have changed between the two branches, unable to diff.
    </details>

Benchmark for macOS-default

    <details open>
      <summary>Click to hide benchmark</summary>
      Benchmarks have changed between the two branches, unable to diff.
    </details>

Benchmark for macOS-metal

    <details open>
      <summary>Click to hide benchmark</summary>
      Benchmarks have changed between the two branches, unable to diff.
    </details>

flaub
flaub reviewed Sep 28, 2023
View reviewed changes
risc0/zkvm/Cargo.toml Outdated
@@ -156,3 +157,6 @@ std = [
# While this is acceptable for most tests, the tests counting cycles and
# segments will fail intermittently.
test-exact-cycles = ["risc0-zkvm-methods/test-exact-cycles"]

# this is a temporary feature that will be removed after proof of fault becomes more stable
enable-fault-proof = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we collapse this into fault-proof?

flaub
flaub reviewed Sep 28, 2023
View reviewed changes
.github/workflows/main.yml Outdated Show resolved Hide resolved
@github-actions
Copy link

Benchmark for Linux-cuda

    <details open>
      <summary>Click to hide benchmark</summary>
      Benchmarks have changed between the two branches, unable to diff.
    </details>

Benchmark for Linux-default a9e435b

Click to hide benchmark
Test Base PR %
fib/100/execute 6.9±0.52ms 6.9±0.43ms 0.00%
fib/100/prove 5.0±0.03s 5.0±0.04s 0.00%
fib/100/total 5.0±0.02s 5.0±0.02s 0.00%
fib/1000/execute 8.2±0.61ms 7.9±0.55ms -3.66%
fib/1000/prove 5.1±0.02s 5.1±0.01s 0.00%
fib/1000/total 5.1±0.02s 5.1±0.02s 0.00%
fib/10000/execute 15.3±0.48ms 14.3±0.47ms -6.54%
fib/10000/prove 21.1±0.16s 21.0±0.11s -0.47%
fib/10000/total 21.1±0.11s 21.0±0.11s -0.47%

Benchmark for macOS-default a9e435b

Click to hide benchmark
Test Base PR %
fib/100/execute 2.8±0.11ms 2.7±0.08ms -3.57%
fib/100/prove 3.6±0.03s 3.6±0.06s 0.00%
fib/100/total 3.6±0.04s 3.6±0.08s 0.00%
fib/1000/execute 3.1±0.11ms 3.1±0.10ms 0.00%
fib/1000/prove 3.7±0.03s 3.7±0.05s 0.00%
fib/1000/total 3.7±0.05s 3.7±0.09s 0.00%
fib/10000/execute 6.2±0.11ms 6.1±0.08ms -1.61%
fib/10000/prove 15.1±0.12s 15.0±0.14s -0.66%
fib/10000/total 15.1±0.20s 15.0±0.10s -0.66%

Benchmark for macOS-metal

    <details open>
      <summary>Click to hide benchmark</summary>
      Benchmarks have changed between the two branches, unable to diff.
    </details>

@github-actions
Copy link

Benchmark for Linux-cuda e2bd50a

Click to hide benchmark
Test Base PR %
fib/100/execute 5.1±0.11ms 5.0±0.10ms -1.96%
fib/100/prove 1558.0±13.71ms 869.9±21.47ms -44.17%
fib/100/total 1594.0±95.18ms 880.3±20.97ms -44.77%
fib/1000/execute 5.7±0.11ms 5.6±0.12ms -1.75%
fib/1000/prove 1596.0±71.69ms 883.8±8.63ms -44.62%
fib/1000/total 1622.2±90.12ms 897.6±26.88ms -44.67%
fib/10000/execute 11.7±0.13ms 11.7±0.13ms 0.00%
fib/10000/prove 4.8±0.01s 3.5±0.03s -27.08%
fib/10000/total 4.8±0.03s 3.5±0.03s -27.08%

Benchmark for Linux-default e2bd50a

Click to hide benchmark
Test Base PR %
fib/100/execute 6.9±0.42ms 6.9±0.49ms 0.00%
fib/100/prove 5.0±0.03s 5.0±0.02s 0.00%
fib/100/total 5.0±0.03s 5.0±0.02s 0.00%
fib/1000/execute 8.0±0.58ms 7.9±0.67ms -1.25%
fib/1000/prove 5.1±0.02s 5.0±0.03s -1.96%
fib/1000/total 5.1±0.03s 5.1±0.02s 0.00%
fib/10000/execute 15.1±0.51ms 14.2±0.56ms -5.96%
fib/10000/prove 20.8±0.12s 20.7±0.09s -0.48%
fib/10000/total 20.9±0.12s 20.7±0.09s -0.96%

Benchmark for macOS-default e2bd50a

Click to hide benchmark
Test Base PR %
fib/100/execute 2.7±0.09ms 2.7±0.13ms 0.00%
fib/100/prove 3.7±0.07s 3.6±0.09s -2.70%
fib/100/total 3.7±0.05s 3.6±0.07s -2.70%
fib/1000/execute 3.1±0.16ms 3.0±0.07ms -3.23%
fib/1000/prove 3.7±0.06s 3.7±0.05s 0.00%
fib/1000/total 3.7±0.06s 3.7±0.05s 0.00%
fib/10000/execute 6.0±0.07ms 5.9±0.12ms -1.67%
fib/10000/prove 15.1±0.15s 15.1±0.21s 0.00%
fib/10000/total 15.0±0.06s 15.0±0.08s 0.00%

Benchmark for macOS-metal e2bd50a

Click to hide benchmark
Test Base PR %
fib/100/execute 2.8±0.05ms 2.8±0.10ms 0.00%
fib/100/prove 804.4±2.29ms 803.8±4.04ms -0.07%
fib/100/total 827.9±5.47ms 827.9±5.34ms 0.00%
fib/1000/execute 3.0±0.10ms 3.0±0.05ms 0.00%
fib/1000/prove 823.6±5.52ms 821.7±2.88ms -0.23%
fib/1000/total 847.9±4.31ms 845.5±5.94ms -0.28%
fib/10000/execute 6.1±0.06ms 6.0±0.06ms -1.64%
fib/10000/prove 3.1±0.01s 3.0±0.01s -3.23%
fib/10000/total 3.1±0.01s 3.0±0.01s -3.23%

@SchmErik SchmErik merged commit 2bf393a into main Sep 29, 2023
20 checks passed
@SchmErik SchmErik deleted the erik/fault-proof-part-1-v2 branch September 29, 2023 04:50
@nategraf nategraf mentioned this pull request Oct 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flaub flaub flaub approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@SchmErik @flaub