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
Preview removed because the pull request was closed.
2026-10-01 22:43 UTC

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.

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: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: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.

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.

david-pl · 2026-09-29T09:48:27Z

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.

I understand, but I don't think this holds if only some of the implementations of the trait actually use an RNG. Making PauliSum submit a dummy rng seems really odd to me and is not worth the abstraction IMO.

Another argument for letting the tableau manage its own rng is that each shot should be seedable and reproducible by a seed. So you'd have to make sure you pass the same rng into the noise as you pass into measurements etc. I think that's easy to forget so having the tableau manage its own rng state centrally is the better approach.

david-pl · 2026-09-29T11:25:00Z

I made some updates here:

Remove PauliErrorFactors, required num::One on Coefficient instead. Note, that Term will still need new AddAssign and MulAssign impls for references. Also, there's a bunch of bugs, see ppvm-sym: Term arithmetic bugs (half, AddAssign<f64>, MulAssign) #232.
Consequently, CorrelatedLossChannel did not get the same treatment as PauliErrorFactors.
Removed the rng from the traits.

If we are unhappy with any of those decisions, we can undo them.

@JonhasA @Roger-luo this PR would be good to go from my side now. What do you think?

JonhasA · 2026-09-30T16:10:32Z

I made some updates here:

Remove PauliErrorFactors, required num::One on Coefficient instead. Note, that Term will still need new AddAssign and MulAssign impls for references. Also, there's a bunch of bugs, see ppvm-sym: Term arithmetic bugs (half, AddAssign, MulAssign) #232.

Consequently, CorrelatedLossChannel did not get the same treatment as PauliErrorFactors.

Removed the rng from the traits.

If we are unhappy with any of those decisions, we can undo them.

@JonhasA @Roger-luo this PR would be good to go from my side now. What do you think?

LGTM! Thank you for the help!

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

Comment thread crates/ppvm-traits-2/src/gates/channel.rs Outdated

Comment thread crates/ppvm-traits-2/src/gates/clifford.rs

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

david-pl mentioned this pull request Sep 29, 2026

ppvm-sym: Term arithmetic bugs (half, AddAssign<f64>, MulAssign) #232

Open

david-pl added 4 commits September 29, 2026 12:41

Remove PauliErrorFactors

18cd822

Remove rng argument from trait

2b5498a

Remove rng from measure & reset too

6195160

Remove doubled - no longer needed

846de3f

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

de4bb73

JonhasA merged commit 7b2e1bc into main Oct 1, 2026
13 checks passed

JonhasA deleted the trait-2/ppvm-traits-2 branch October 1, 2026 22:43

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!

Uh oh!

david-pl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

david-pl Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

david-pl Sep 22, 2026

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!

Uh oh!

Uh oh!

david-pl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

david-pl Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

david-pl Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

JonhasA Sep 22, 2026

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!

Uh oh!

david-pl Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Roger-luo commented Sep 22, 2026

Uh oh!

david-pl commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

david-pl commented Sep 29, 2026

Uh oh!

JonhasA commented Sep 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Sep 10, 2026 •

edited

Loading

david-pl commented Sep 29, 2026 •

edited

Loading