PPVM-traits-2 Patch by JonhasA · Pull Request #219 · QuEraComputing/ppvm

JonhasA · 2026-09-10T14:37:50Z

A patch to move the changes to ppvm-trait from roger's PR #204

Additionally, the following changes were made:

Added arithmetic module for defining traits to numeric types
added gates module for defining traits for Clifford/ channel / measure/ rotation ops
container module now has additional files related to storage or engine configuration
Moved PREFER_MOVED_RKEY outside of coefficient and into storage.rs in containers/. Engine will configure the key rather than having coefficient module be responsible for it.
Added a LossState trait to be associated with word sites rather than having it in PauliBits.
Isolated functionality related to fermionic factors into fermion_factor.rs
Pauli type is used explicitly for rotation operations
Moves container tests beside their implementations and adds regression coverage for batch hash invalidation and Pauli-channel factors.
Moves pauli_error_factors into the optional PauliErrorFactors channel capability, preserving the generic default and numeric specializations.

…em for ppvm. Focused on moving files to more isolated modules to have better appreciation on module responsibility

…Action and FermionSite

github-actions

👋 Thanks for opening your first pull request against PPVM!

A quick note on contribution terms: by submitting this PR you
agree that your contribution is licensed under the
Apache License 2.0
and that you accept the
PPVM Contributor License Agreement.
Please skim those before a maintainer reviews — opening this PR
counts as your acceptance.

A few things that will speed up review:

Read CONTRIBUTING.md
for the workflow, build commands, and style notes.
Run prek run --all-files locally; CI runs the same checks.
Use Conventional Commits
for commit messages.

We'll get to your PR as soon as we can. Thanks for contributing!

github-actions · 2026-09-10T14:50:30Z

PR Preview Action v1.8.1
🚀 View preview at https://QuEraComputing.github.io/ppvm/pr-preview/pr-219/
Built to branch `gh-pages` at 2026-09-28 15:49 UTC. Preview will be ready when the GitHub Pages deployment is complete.

david-pl

Overall a solid first breakdown of the huge PR, but I think we should make some changes here.

Also, on a more general note: while I get that proving things with lean is beneficial, I think this introduces unneeded complexity at points. For example, to fulfill criteria so this is provably a ring we introduce some traits that are sometimes a bit odd, e.g. Halvable.
cc @Roger-luo

david-pl · 2026-09-15T09:38:20Z

+impl Halvable for f64 {
+    #[inline]
+    fn half(&self) -> Self {
+        *self / 2.0


I'm surprised that this holds up in the exact x.half() + x.half() == x condition.

Actually, there is an edge case here: this doesn't hold for f64::NAN, which is a perfectly valid f64, but x.half() == x if x is NAN.

I'm not saying this bothers me overly much, but as I said I'd also be happy not to go the full formal ring way anyway.

…of individual bits.

…d structs can be added later if deemed necessary

…anization. So, we now have RotationOne, RotationOneBatch, RotationTwo, and RotationTwoBatch

…operations with respect to the consumer. This allows sepcialized or optimal implementation dependent on the consumer, rather than just inheriting it.

Roger-luo · 2026-09-20T23:32:57Z

Also, on a more general note: while I get that proving things with lean is beneficial, I think this introduces unneeded complexity at points. For example, to fulfill criteria so this is provably a ring we introduce some traits that are sometimes a bit odd, e.g. Halvable.

For reference, One use case is symbolic expressions. The minimum requirement of a symbolic expression is that it can be divided by half, and it doesn't matter what number that half comes from.

Roger-luo · 2026-09-20T23:40:53Z

+        &mut self,
+        qubit0: usize,
+        qubit1: usize,
+        p: [C; 3],


This might need similar treatment, like the pauli channrl error coefficient factors, too, because we assume the three parameters also satisfy the normalization condition, right?

Roger-luo · 2026-09-20T23:41:35Z

+}
+
+/// Batched Clifford gates: apply the same gate to many qubits in one call.
+pub trait CliffordBatch: Clifford {


I was thinking if there is a way to generalize this based on the Clifford trait, but I didn't really find a good way to define a broadcast trait in general, so I ended up doing this.

Related question: do we have any consumers of this trait that don't run a custom impl? Do we really need the fallback loops?

I think technically you could have a batch version of the gate implementation, with the consideration of SIMD and parallel operation. For example, if you are applying a batch with a static, known size ahead of time, say it is an X gate, you could come up with the flipping rules ahead of time that modify a batch of bits instead of a single bit in advance.

In principle, that would be faster because then you can utilize SIMD operations and parallel access to your containers.

david-pl

Some minor stuff I still found, and some responses to @Roger-luo here. But it's almost there.

david-pl · 2026-09-21T06:48:11Z

+
+    ///  Accumulates a borrowed coefficient.
+    #[inline]
+    fn add_assign_ref(&mut self, rhs: &Self) {


My point before was to require that trait and then remove this method. It's much more natural to just write *self += rhs and it's also done for the most part (which is why this method was only used once). So, just remove it?

david-pl · 2026-09-21T06:53:10Z

+}
+
+/// Batched Clifford gates: apply the same gate to many qubits in one call.
+pub trait CliffordBatch: Clifford {


Related question: do we have any consumers of this trait that don't run a custom impl? Do we really need the fallback loops?

david-pl · 2026-09-21T06:56:39Z

+            0 => Phase::Pos1,
+            1 => Phase::PosI,
+            2 => Phase::Neg1,
+            _ => Phase::NegI,


This could silently introduce false results if there's a bug somewhere such that k & 3 > 3.

in #204, Sum<S, P> overrides only cnot_many and inherits the loops for x_many, y_many, and the other _many operators.

…hashing, gathering keys and KeyColumnMut for key construction, resevrqation, and removal

david-pl

The left-over clones need to go. And I just realized that there's no rng on PauliSum. Why was it added to the trait @Roger-luo ?

david-pl · 2026-09-22T08:20:06Z

+    fn accumulate_batch(&mut self, terms: &TermBatch<K, C>) {
+        for (k, c) in terms.iter() {
+            self.entry(k.clone())
+                .and_modify(|e| *e += c.clone())


This no longer needs to c.clone() now due to the AddAssign bound we added.

david-pl · 2026-09-22T08:20:38Z

+        self.as_slice()
+            .iter()
+            .find(|(k, _)| k == key)
+            .map(|(_, c)| c.clone())


c.clone() is no longer needed because of the AddAssign bound.

david-pl · 2026-09-22T08:26:30Z

+impl Halvable for f64 {
+    #[inline]
+    fn half(&self) -> Self {
+        *self / 2.0


Actually, there is an edge case here: this doesn't hold for f64::NAN, which is a perfectly valid f64, but x.half() == x if x is NAN.

I'm not saying this bothers me overly much, but as I said I'd also be happy not to go the full formal ring way anyway.

david-pl · 2026-09-22T08:29:32Z

+        &mut self,
+        qubit: usize,
+        probabilities: [C; 3],
+        rng: &mut R,


Actually, PauliSum is deterministic in noise channels. Why are we passing in an rng here? PauliSum doesn't have one. @Roger-luo ?

david-pl · 2026-09-22T08:31:26Z

+/// Correlated two-qubit loss channel.
+pub trait CorrelatedLossChannel<C: Coefficient> {
+    /// Apply correlated loss: `p[0]` loses both qubits when both are present;
+    /// `p[1]` loses either one when both are present; `p[2]` loses the remaining


There was a recent fix to the correlated loss convention here: #205

This docstring seems to have regressed now, probably because #204 was based off the old state.

Roger-luo · 2026-09-22T13:31:38Z

good catch, this is a part I'm not sure - I moved rng out because it's an external state not necessarily (should) be managed by the PauliSum/tableau which is a quantum state, I think it's more natural to have it passed as a argument and let external/global managing it so it's more explicit who is sharing the RNG with what seed.

On the other hand, RNG is technically part of the entire execution state, so it could make sense to let the state object carrying it.

JonhasA added 3 commits September 9, 2026 10:52

initial commit of breaking up roger's PR on rebuilding the trait syst…

2a7f074

…em for ppvm. Focused on moving files to more isolated modules to have better appreciation on module responsibility

added ppvm-traits-2 to workspace and forgot to factor back in fermion…

fab45e2

…Action and FermionSite

ran cargo fmt

ebb25e0

JonhasA requested review from Roger-luo and david-pl September 10, 2026 14:37

github-actions Bot reviewed Sep 10, 2026

View reviewed changes

adding missing header to fermion_factor

5e2e779

david-pl requested changes Sep 16, 2026

View reviewed changes

JonhasA added 9 commits September 16, 2026 12:02

condensed comments to maximum 3 lines or so

17d676e

removed clippy expections, and uses toggle_x, toggle_z masks instead …

0c358a7

…of individual bits.

removed orphaned traits and structs that don't use it. Such traits an…

24dbeed

…d structs can be added later if deemed necessary

modified exponent(self)-> u8 in algebra.rs to just return self as u8

140ae62

merged CliffordExtension into Clifford

26c2d12

Merge branch 'main' into trait-2/ppvm-traits-2

a14a33a

Added AddAssign and MulAssign bounds to coefficient to avoid cloning

c26c14b

moved RotationOne and RotationTwo to match Clifford/CliffordBatch org…

5e066c7

…anization. So, we now have RotationOne, RotationOneBatch, RotationTwo, and RotationTwoBatch

removed BlanketClifford as we force end users to define the clifford …

db46e67

…operations with respect to the consumer. This allows sepcialized or optimal implementation dependent on the consumer, rather than just inheriting it.

Roger-luo reviewed Sep 20, 2026

View reviewed changes

Roger-luo requested a review from david-pl September 20, 2026 23:42

david-pl reviewed Sep 21, 2026

View reviewed changes

JonhasA added 3 commits September 21, 2026 23:57

seperated KeyColumn into two seperate traits. KeyColumn for reading, …

cf2cbc3

…hashing, gathering keys and KeyColumnMut for key construction, resevrqation, and removal

made pattern matching explicit for k & 3 in algebra.rs

bb21999

removed add_assign_ref as *self += rhs is more natural

6341d2c

david-pl reviewed Sep 22, 2026

View reviewed changes

JonhasA added 2 commits September 28, 2026 10:03

another round of responses

2bb8f63

other round of cleanup given feedback

fbb95dd

Conversation

JonhasA commented Sep 10, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Built to branch gh-pages at 2026-09-28 15:49 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

david-pl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Roger-luo commented Sep 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-pl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-pl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Roger-luo commented Sep 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Sep 10, 2026 •

edited

Loading

Built to branch `gh-pages` at 2026-09-28 15:49 UTC.
Preview will be ready when the GitHub Pages deployment is complete.