Return the signed half-angle from Angle::half_angle_sin_cos for negative angles - #604
Merged
Conversation
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #599. Split out of #601 so it can land on its own: it is a pre-existing defect, independent of the T-gate convention work, and much smaller.
The defect
Angle::half_angle_sin_coscomputed the half-angle by halving the stored fixed-point fraction:Anglestores an unsigned fraction of a full turn in[0, 2^BITS), so an angle with a negative principal value is stored wrapped, near the top of the range. Halving that stored value gives the true half-angle plus pi, so both returned components come back negated:Positive principal values were correct, so the defect was invisible to any test exercising only positive angles.
Why it went unnoticed
Rotation gates are built as
RZ(theta) = cos(theta/2) I - i sin(theta/2) Z. Negating both half-angle components yields-RZ(theta): a global phase of-1. That is unobservable in probabilities, in density-matrix evolution, and in any comparison that quotients global phase -- which is what PECOS's state-comparison helpers do. It becomes observable only where absolute amplitudes matter or a tracked global phase is exposed.Affected callers on
devcrates/pecos-simulators/src/stab_vec.rs-- non-CliffordRZmaterialisation, for negative anglesexp/pecos-stab-tn/src/stab_mps/mast.rs-- itscorrection_anglecan be negative, so that branch was applying-RZ(phi)Both are fixed by this change; neither needed an edit of its own.
The fix
Corrected in the helper rather than at the call sites, and kept in fixed point. The sign test is made on the fraction, not on a converted
f64, which is deliberate and strictly more accurate near half a turn: fractions one unit aboveHALF_TURNhave a negative principal value but round to exactlypiinf64, so anf64-based test would call them positive. The doc comment says not to "simplify" this into agreement withto_radians_signed.The boundary is strictly greater than half a turn, so exactly half a turn keeps the
+piprincipal value, matchingto_radians_signed's(-pi, pi]range. This matters:RZ(pi)must be-iZ, and using the top bit alone would have negated it to+iZ.Verification
cargo test -p pecos-core-- passes, including a new generic test overAngle8,Angle16,Angle32,Angle64andAngle128covering positive, negative, zero, exact Clifford, half-turn and near-full-turn inputscargo test -p pecos-simulators-- passescargo test -p pecos-stab-tn --lib-- 341 passed--locked, pluspecos --no-default-features),cargo fmt --checkand pre-commit -- cleanNo existing expected value needed changing: the affected results were off by a global phase that no current test pinned.
Relationship to #601
#601 contains this same fix, because a correct
Tdgdepends on it. If this lands first, #601 should be rebased and the overlap will drop out.