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.

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

clarifying image vs imageid #875

Merged
merged 2 commits into from
Sep 14, 2023
Merged

clarifying image vs imageid #875

merged 2 commits into from
Sep 14, 2023

Conversation

pdg744
Copy link
Contributor

@pdg744 pdg744 commented Sep 13, 2023

Current documentation suggests verification algorithm takes the entire binary file as input.
This PR clarifies that it just needs the ImageID

@pdg744
Copy link
Contributor Author

pdg744 commented Sep 13, 2023

@tzerrell is this way of including links fair game here?

Copy link
Member

@tzerrell tzerrell left a comment

Choose a reason for hiding this comment

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

Yes, this is fine here. If you ever want to check for yourself, you can build the docs locally with cargo doc from the risc0 root directory and open the generated webpage in a browser (in target/doc ... e.g. on my system the relevant file is at file:///Users/tzerrell/code/risc0/target/doc/risc0_zkvm/struct.Receipt.html). I'm also happy to answer too though!

@mothran
Copy link
Contributor

mothran commented Sep 14, 2023

Yes, this is fine here. If you ever want to check for yourself, you can build the docs locally with cargo doc from the risc0 root directory and open the generated webpage in a browser (in target/doc ... e.g. on my system the relevant file is at file:///Users/tzerrell/code/risc0/target/doc/risc0_zkvm/struct.Receipt.html). I'm also happy to answer too though!

Additionally: cargo doc --no-deps -p <crate-name> to make the build way faster!

@flaub
Copy link
Member

flaub commented Sep 14, 2023

and there's a nice --open flag

@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 2a8a8fd

Click to hide benchmark
Test Base PR %
fib/100/execute 6.8±0.46ms 6.7±0.41ms -1.47%
fib/100/prove 5.0±0.03s 5.0±0.03s 0.00%
fib/100/total 5.0±0.02s 5.0±0.02s 0.00%
fib/1000/execute 8.0±0.69ms 7.9±0.42ms -1.25%
fib/1000/prove 5.0±0.03s 5.0±0.02s 0.00%
fib/1000/total 5.0±0.02s 5.0±0.01s 0.00%
fib/10000/execute 14.6±0.56ms 14.4±0.46ms -1.37%
fib/10000/prove 20.8±0.09s 20.8±0.12s 0.00%
fib/10000/total 20.9±0.06s 20.8±0.10s -0.48%

Benchmark for macOS-default 2a8a8fd

Click to hide benchmark
Test Base PR %
fib/100/execute 2.8±0.17ms 2.7±0.11ms -3.57%
fib/100/prove 3.6±0.05s 3.6±0.07s 0.00%
fib/100/total 3.6±0.05s 3.6±0.05s 0.00%
fib/1000/execute 3.1±0.18ms 3.1±0.16ms 0.00%
fib/1000/prove 3.7±0.08s 3.7±0.04s 0.00%
fib/1000/total 3.7±0.03s 3.7±0.07s 0.00%
fib/10000/execute 6.0±0.16ms 5.8±0.10ms -3.33%
fib/10000/prove 15.1±0.18s 15.0±0.07s -0.66%
fib/10000/total 15.0±0.12s 15.0±0.11s 0.00%

Benchmark for macOS-metal 2a8a8fd

Click to hide benchmark
Test Base PR %
fib/100/execute 2.8±0.13ms 2.7±0.20ms -3.57%
fib/100/prove 804.7±4.17ms 800.3±3.77ms -0.55%
fib/100/total 824.2±3.74ms 821.5±5.26ms -0.33%
fib/1000/execute 3.1±0.07ms 3.0±0.07ms -3.23%
fib/1000/prove 822.5±3.58ms 818.7±2.80ms -0.46%
fib/1000/total 844.1±3.52ms 838.0±6.53ms -0.72%
fib/10000/execute 5.8±0.07ms 5.8±0.05ms 0.00%
fib/10000/prove 3.1±0.01s 3.1±0.01s 0.00%
fib/10000/total 3.1±0.01s 3.1±0.01s 0.00%

@flaub flaub enabled auto-merge (squash) September 14, 2023 16:46
@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 a839d05

Click to hide benchmark
Test Base PR %
fib/100/execute 2.8±0.10ms 2.7±0.20ms -3.57%
fib/100/prove 3.7±0.05s 3.6±0.06s -2.70%
fib/100/total 3.6±0.06s 3.6±0.07s 0.00%
fib/1000/execute 3.0±0.09ms 3.0±0.02ms 0.00%
fib/1000/prove 3.7±0.07s 3.7±0.07s 0.00%
fib/1000/total 3.7±0.07s 3.7±0.06s 0.00%
fib/10000/execute 5.9±0.05ms 5.7±0.12ms -3.39%
fib/10000/prove 15.1±0.11s 15.0±0.12s -0.66%
fib/10000/total 15.2±0.21s 15.0±0.11s -1.32%

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 merged commit 54936aa into main Sep 14, 2023
20 checks passed
@flaub flaub deleted the paulg/crate-doc-fixes branch September 14, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants