Summary
#205 adds admissibility checks for the correlated-loss channel — p[0], p[1] >= 0, p[0] + 2·p[1] <= 1, p[2] ∈ [0, 1] — but they only protect developers, not users. Three gaps, in increasing order of user impact.
1. debug_assert is compiled out of release wheels
The guards are debug_assert!, so a released ppvm wheel performs no validation at all. Pass an inadmissible triple and you silently get a non-completely-positive map: negative branch weights, expectation values outside [-1, 1], and a mixture that renormalizes nonsense into something plausible-looking. Measured before #205: p = [5, -3, 17] returned a coefficient of +2.0 from LossyPauliSum, and the mixture turned a negative survivor weight into "both lost with probability 1".
2. When it does fire, it's a PanicException
Under a maturin develop install the guard surfaces as pyo3_runtime.PanicException, not a ValueError. That's not catchable as an ordinary Python error, gives no parameter context, and reads as an internal crash rather than "you passed an invalid probability". Validation belongs at the binding layer, where it can raise a real exception with a useful message, on every build profile.
3. ppvm-pauli-sum cannot guard at all
Sum::correlated_loss_channel is generic over Coefficient, and that trait carries no ordering — Complex<f64> isn't PartialOrd — so the region cannot be tested directly. #205 works around this with a triangle-inequality trick on magnitude(), but that has its own hole: ppvm-sym-2::Term::magnitude documents an explicit exemption returning f64::INFINITY for non-constant forms, so a symbolic probability can't be validated either way.
Net effect today: LossyPauliSum silently produces negative coefficients for exactly the triples on which GeneralizedTableau now raises. The backends are consistent on valid input after #205 and inconsistent on invalid input.
Suggested direction
- Validate in the PyO3 binding layer and raise
ValueError with the offending parameters, so it works in release wheels and is catchable.
- Keep the Rust
debug_asserts as a developer backstop for direct Rust callers.
- For the coefficient-generic path, either add an opt-in
TryIntoProbability-style capability that concrete coefficient types implement, or accept that generic coefficients are unvalidated and say so in the trait docs rather than implying a guard exists.
- Worth applying to the other probabilistic channels at the same time —
pauli_error, two_qubit_pauli_error and depolarize* currently assert only that each entry is in [0, 1], so sum p = 3 passes.
Context: the missing positivity constraint was also flagged in review on #38 ("clarify any constraints such as 2*p[1] + p[0] <= 1 needed to keep coefficients non-negative") and not applied at the time. #205 lands the constraint; this issue is about making it reach users.
🤖 Generated with Claude Code
Summary
#205 adds admissibility checks for the correlated-loss channel —
p[0], p[1] >= 0,p[0] + 2·p[1] <= 1,p[2] ∈ [0, 1]— but they only protect developers, not users. Three gaps, in increasing order of user impact.1.
debug_assertis compiled out of release wheelsThe guards are
debug_assert!, so a releasedppvmwheel performs no validation at all. Pass an inadmissible triple and you silently get a non-completely-positive map: negative branch weights, expectation values outside[-1, 1], and a mixture that renormalizes nonsense into something plausible-looking. Measured before #205:p = [5, -3, 17]returned a coefficient of+2.0fromLossyPauliSum, and the mixture turned a negative survivor weight into "both lost with probability 1".2. When it does fire, it's a
PanicExceptionUnder a
maturin developinstall the guard surfaces aspyo3_runtime.PanicException, not aValueError. That's not catchable as an ordinary Python error, gives no parameter context, and reads as an internal crash rather than "you passed an invalid probability". Validation belongs at the binding layer, where it can raise a real exception with a useful message, on every build profile.3.
ppvm-pauli-sumcannot guard at allSum::correlated_loss_channelis generic overCoefficient, and that trait carries no ordering —Complex<f64>isn'tPartialOrd— so the region cannot be tested directly. #205 works around this with a triangle-inequality trick onmagnitude(), but that has its own hole:ppvm-sym-2::Term::magnitudedocuments an explicit exemption returningf64::INFINITYfor non-constant forms, so a symbolic probability can't be validated either way.Net effect today:
LossyPauliSumsilently produces negative coefficients for exactly the triples on whichGeneralizedTableaunow raises. The backends are consistent on valid input after #205 and inconsistent on invalid input.Suggested direction
ValueErrorwith the offending parameters, so it works in release wheels and is catchable.debug_asserts as a developer backstop for direct Rust callers.TryIntoProbability-style capability that concrete coefficient types implement, or accept that generic coefficients are unvalidated and say so in the trait docs rather than implying a guard exists.pauli_error,two_qubit_pauli_erroranddepolarize*currently assert only that each entry is in[0, 1], sosum p = 3passes.Context: the missing positivity constraint was also flagged in review on #38 ("clarify any constraints such as
2*p[1] + p[0] <= 1needed to keep coefficients non-negative") and not applied at the time. #205 lands the constraint; this issue is about making it reach users.🤖 Generated with Claude Code