Repository navigation
Fix non-equivalence in toffoli_flip_greedy when X gate sits between CCZs (fixes #230) - #232
Open
ChethanaMallisetty21 wants to merge 2 commits into
Conversation
…uantum-compiler#230) - Remove GateType::x from Graph::moveable() so that rotation merging does not traverse across X gates. - When an X gate is encountered on a wire, the CNOT/rotation block terminates on that wire, preventing unsound cancellation of Rz rotations across X. - Add regression test in test_issue_230.cpp and counterexample circuit ccz_x_ccz.qasm.
Author
|
@xumingkuan @tarassh pr is ready for review |
Collaborator
|
I think this is not the ideal fix -- as you said, X R_z(\theta) X = R_z(-\theta). So we could just update |
|
Thanks for the fix. Two observations from my runs:
The CNOT count is the same; the extra gates are Rz rotations that no longer merge across X. This seems to match @xumingkuan's point that tracking the sign flip of X (X Rz(t) X = Rz(-t)) would avoid excluding X altogether.
|
- Keep GateType::x moveable in Graph::moveable() as suggested by maintainers. - In Graph::rotation_merging(), track affine sign flips (X R_z(\theta) X = R_z(-\theta)) by reserving an extra affine bit in bitmasks (num_qubits + 1). On encountering X, flip the affine bit; on CX, propagate it. - Fix rotation angles in RuleParser::ccz_cx_rz_rules() to 0.125pi to account for RZGate parameter halving in Quartz. - Fix CircuitSeq::evaluate() parameter handling in test_issue_230 to prevent segfaults and replace asserts with explicit runtime error checks so tests are not compiled out under -DNDEBUG. - Restores master preprocessed gate counts on adder_8 (732) and mod_red_21 (236), while preserving phase equivalence on issue quantum-compiler#230 counterexamples.
Author
|
Hi @xumingkuan @tarassh, thank you for the feedback and pointers! I have addressed all the review items in commit 1. Affine Sign Flip Tracking Across X Gates
2. Restored Benchmark Optimization CountsWith X gates moveable and affine sign tracking in place, valid rotation merges across X are restored, matching mainline master (
3. Fixes in
|
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.
Summary
Fixes #230.
This PR fixes a bug in offoli_flip_greedy where circuits containing Pauli-$ gates between $ gates produced non-equivalent circuits during preprocessing due to improper rotation merging across $.
Root Cause
In src/quartz/tasograph/tasograph.cpp, Graph::moveable(GateType tp) returned rue for GateType::x.
Rotation merging operates on affine-linear${CNOT, R_z}$ phase polynomial blocks. However, Pauli-$ anticommutes with $ in phase:$+\pi/4$ ) and CCZ #2 with Rule 1 ($-\pi/4$ ), all rotations across $ canceled each other to zero ($\theta + (-\theta) = 0$ ). This collapsed the phase block into a pure classical CNOT/X permutation, erasing the $ relative phase.
X R_z(\theta) X = R_z(-\theta)
and propagates as target $ gates across CNOTs ( CX_{1,2} = CX_{1,2} X_1 X_2$). Treating $ as moveable allowed Graph::expand() to commute across $ into downstream $ decompositions. When CCZ #1 was decomposed with Rule 0 (
This also resolves the mystery of why exactly 4 benchmarks in Table 2 of the PLDI 2022 Quartz paper (mod_mult_55,
c_adder_6, csum_mux_9, and qcla_mod_7) failed equivalence verification: each of those 4 contains \dots X \dots CCZ$ sequences.
Changes
Removed p == GateType::x from �ool Graph::moveable(GateType tp). Graph::expand() now respects $ as a phase block boundary (just like $).
Added the minimal 3-gate counterexample circuit from
toffoli_flip_greedyreturns a non-equivalent circuit when an X gate sits between two CCZs #230.Added an automated regression test that:
Registered est_issue_230 executable.
Verification
ot_equivalent
c_adder_6: previously$\to$ now equivalent_up_to_global_phase
ot_equivalent