latticesurgery-com / lattice-surgery-compiler

Lattice surgery quantum error correction compiler
https://latticesurgery.com
GNU Lesser General Public License v2.1
46 stars 6 forks source link

[BUG-FIX] T Gate missng in RZ Gate definition #273

Closed isolatedinformation closed 2 years ago

isolatedinformation commented 2 years ago

Issue Description

For both Fraction(1,2) and Fraction(1,4), the to_clifford_plus_t() method decomposes the RZ rotation to the S gate for both the fractions, instead of decomposing to the S and T gates respectively. This PR fixes this :)

How to Reproduce

Code Snippet

TEST_RZ_GATE = """OPENQASM 2.0;
include "qelib1.inc";
qreg q0[4];
h q0[3];
barrier q0[0],q0[1],q0[2],q0[3];
rz(pi/2) q0[1];
rz(pi/4) q0[0];
"""

circ = GatesCircuit().from_qasm(TEST_RZ_GATE)
print("Gate Sequence before conversion to Clifford+T")
for gate in circ.gates:
    print(gate)
print("\nGate sequence after conversion to Clifford T ")
for gate in circ.to_clifford_plus_t().gates:
    print(gate)

Error Output

Gate Sequence before conversion to Clifford+T
H(target_qubit=3)
RZ(target_qubit=1, phase=Fraction(1, 2))
RZ(target_qubit=0, phase=Fraction(1, 4))

Gate sequence after conversion to Clifford T
H(target_qubit=3)
S(target_qubit=1)
S(target_qubit=0)

Corrected Output (in this pr)

Gate Sequence before conversion to Clifford+T
H(target_qubit=3)
RZ(target_qubit=1, phase=Fraction(1, 2))
RZ(target_qubit=0, phase=Fraction(1, 4))

Gate sequence after conversion to Clifford T
H(target_qubit=3)
S(target_qubit=1)
T(target_qubit=0)
isolatedinformation commented 2 years ago

So pytest did not pick this up as an error because the test classes were defined with the name GateTest instead ofTestGate https://stackoverflow.com/questions/34363388/pytest-no-tests-ran! Added fixes for the tests also

isolatedinformation commented 2 years ago

@gwwatkin @alexnguyenn The test actions for pytest did not run properly for this! I stopped it after it ran for 20 minutes

gwwatkin commented 2 years ago

@isolatedinformation did you run pytest --update-snapshot?

gwwatkin commented 2 years ago

And yes we need to fix this, when certain long snapshots don't match it seems to hang, but that is the whole point of snapshot tests, they check for regressions.

@alexnguyenn can we set a timeout for a couple minutes?

isolatedinformation commented 2 years ago

yeah i did run to update the snapshot and tests ran locally, it just did not on the actions

gwwatkin commented 2 years ago

@isolatedinformation I think you need to commit the changes to the snapshot files, I don't see any of them changed in the diff

codecov[bot] commented 2 years ago

Codecov Report

Merging #273 (165561c) into dev (d59f447) will increase coverage by 0.08%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #273      +/-   ##
==========================================
+ Coverage   71.89%   71.98%   +0.08%     
==========================================
  Files          31       31              
  Lines        2288     2288              
==========================================
+ Hits         1645     1647       +2     
+ Misses        643      641       -2     
Impacted Files Coverage Δ
src/lsqecc/gates/gates.py 100.00% <100.00%> (+4.08%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d59f447...165561c. Read the comment docs.