Repository navigation
[codex] Specialize ppvm-runtime depolarizing noise fast paths #54
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # runtime-micro-overall | ||
|
|
||
| ## Target | ||
|
|
||
| Improve the overall `ppvm-runtime` `micro` benchmark suite, using the first fresh full-suite run in this session as the baseline. | ||
|
|
||
| ## Architecture Notes | ||
|
|
||
| - The slowest representative microbenchmarks in the baseline run clustered around the shared `PauliSum` transform paths: `clifford/single/h` at `1.053 µs`, `clifford/two-qubit/cnot` at `1.050 µs`, and `scaling/cnot/1000` at `1.418 µs`. | ||
| - A quick ad-hoc profile showed cloning was not the dominant cost for representative operations on the benchmark state shape (`clone_only_ns≈72-83ns` versus total operation timings in the `170-295ns` range in the profiler), so the first optimization hypothesis focused on transform mechanics rather than clone elimination. | ||
| - Single-word profiling did not show `rehash()` dominating gate cost, so rehash-specific work was deprioritized. | ||
|
|
||
| ## Iterations | ||
|
|
||
| ### owned-bijective-clifford-transform | ||
|
|
||
| - Status: discard | ||
| - Hypothesis: Clifford conjugation is bijective, so consuming owned map entries should avoid clone-heavy `map_add` work and speed up `h`/`cnot`/related scaling benchmarks. | ||
| - Result: strong regression. | ||
| - Evidence: | ||
| - `clifford/single/h`: `1.053 µs -> 1.362 µs` | ||
| - `clifford/two-qubit/cnot`: `1.050 µs -> 1.441 µs` | ||
| - `scaling/cnot/1000`: `1.418 µs -> 2.533 µs` | ||
| - Takeaway: the current `map_add` path plus existing map behavior is materially better than the owned-entry transform on this workload. | ||
|
|
||
| ### noise-factor-specialization | ||
|
|
||
| - Status: keep | ||
| - Hypothesis: the noise group spends avoidable time in repeated branchy factor selection and, for `depolarize2`, unnecessary construction of a 15-entry probability array followed by the generic two-qubit channel. | ||
| - Result: keep. | ||
| - Implemented: | ||
| - Simplified `pauli_error` and `depolarize` to use bit-based Pauli classification and precomputed single-qubit factors. | ||
| - Replaced `depolarize2` with the closed-form two-qubit depolarizing factor `1 - 16p/15` for any non-identity two-qubit Pauli term. | ||
| - Reverted the attempted generic `two_qubit_pauli_error` factor table after it added too much fixed overhead. | ||
| - Evidence from the final full-suite run: | ||
| - `noise/pauli_error`: `279.81 ns -> 213.57 ns` | ||
| - `noise/depolarize`: `272.73 ns -> 217.39 ns` | ||
| - `noise/depolarize2`: `509.21 ns -> 269.71 ns` | ||
| - `noise/amplitude_damping`: `524.70 ns -> 458.56 ns` | ||
| - `noise/two_qubit_pauli_error`: `523.07 ns -> 457.41 ns` | ||
|
|
||
| ## Current Best | ||
|
|
||
| - Keep the noise specialization in `crates/ppvm-runtime/src/sum/noise.rs`. | ||
| - Discard the owned-entry Clifford transform idea unless new profiling reveals a different map implementation bottleneck. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| task = "runtime-micro-overall" | ||
| branch = "codex/autotune-runtime" | ||
| benchmark = "cargo bench -p ppvm-runtime --bench micro -- --noplot" | ||
|
|
||
| [[result]] | ||
| name = "baseline" | ||
| status = "baseline" | ||
| notes = "Initial full micro-suite run used to choose the target. Absolute medians copied from the first fresh run in this session." | ||
|
|
||
| [result.metrics] | ||
| noise_pauli_error_ns = 279.81 | ||
| noise_two_qubit_pauli_error_ns = 523.07 | ||
| noise_depolarize_ns = 272.73 | ||
| noise_depolarize2_ns = 509.21 | ||
| noise_amplitude_damping_ns = 524.70 | ||
| clifford_h_ns = 1053.30 | ||
| clifford_cnot_ns = 1050.30 | ||
| scaling_cnot_1000_ns = 1418.00 | ||
|
|
||
| [[result]] | ||
| name = "owned-bijective-clifford-transform" | ||
| status = "discard" | ||
| notes = "Replaced map_add-based Clifford transforms with an owned-entry bijective transform. This regressed representative benchmarks badly and was reverted." | ||
|
|
||
| [result.metrics] | ||
| clifford_h_ns = 1362.00 | ||
| clifford_cnot_ns = 1441.20 | ||
| scaling_cnot_1000_ns = 2533.00 | ||
|
|
||
| [[result]] | ||
| name = "noise-factor-specialization" | ||
| status = "keep" | ||
| notes = "Kept the single-qubit noise fast path and specialized depolarize2 to the closed-form uniform two-qubit depolarizing factor." | ||
|
|
||
| [result.metrics] | ||
| noise_pauli_error_ns = 213.57 | ||
| noise_two_qubit_pauli_error_ns = 457.41 | ||
| noise_depolarize_ns = 217.39 | ||
| noise_depolarize2_ns = 269.71 | ||
| noise_amplitude_damping_ns = 458.56 | ||
| clifford_h_ns = 1022.50 | ||
| clifford_cnot_ns = 1080.40 | ||
| scaling_cnot_1000_ns = 1197.10 |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pauli_errorbehavior was refactored to rely on bit-based Pauli classification (pauli_code+get_lbit), but there is no unit test that directly exercisesPauliError::pauli_error(unlikedepolarize/depolarize2). Adding a regression test that checks the scaling for I/X/Y/Z (and that L is unchanged) would help catch any future mismatches between the bit encoding and thePaulienum mapping.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a regression test
test_pauli_errorincrates/ppvm-runtime/tests/noise.rs(commit050cb02) that verifies the coefficient scaling for each Pauli at the target qubit:1 - 2*py - 2*pz1 - 2*px - 2*pz1 - 2*px - 2*pyget_lbitguard)