Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Mar 22, 2022
Merged

Conversation

isolatedinformation
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

isolatedinformation commented Mar 21, 2022

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

@gwwatkin
Copy link
Member

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

@gwwatkin
Copy link
Member

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
Copy link
Contributor Author

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

@gwwatkin
Copy link
Member

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

@github-actions github-actions bot added the testing All things test-related label Mar 22, 2022
@codecov
Copy link

codecov bot commented Mar 22, 2022

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%) ⬆️

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.

@isolatedinformation isolatedinformation merged commit ca085dd into dev Mar 22, 2022
@isolatedinformation isolatedinformation deleted the gates-bug-fix branch March 22, 2022 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler testing All things test-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants