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

Add support for most OpenQASM 2 and 3 standard gates. #156

Merged
merged 19 commits into from
Oct 5, 2023

Conversation

dlyongemallo
Copy link
Contributor

@dlyongemallo dlyongemallo commented Sep 16, 2023

Adds support for the most common OpenQASM 2 and 3 standard library gates, and a unit test to ensure that the implementations in pyzx and qiskit are the same up to a global phase.

This PR fixes #153 and #158, adds tests to verify #102 and #103, and should now go most of the way towards completing #87 and #116.

There are number of differences between pyzx's and qiskit's interpretation of qasm gates, leading to user confusion. This PR adds a unit test to document these differences. Users or developers may trigger a test failure to see an error message showing the difference between the pyzx and qiskit versions of a qasm gate.
The `p` gate was added as a standard gate to `qelib1.inc` in Qiskit/qiskit#4765.
@dlyongemallo dlyongemallo changed the title Document qasm gate semantics when imported into pyzx, with a unit test. Add support for most OpenQASM 2 and 3 standard gates. Sep 18, 2023
@dlyongemallo
Copy link
Contributor Author

@jvdwetering I'm slightly unsure about whether to_basic_gates() for crx and rxx are correct. Does the list which is returned have to consist only of gates for which to_basic_gates() return themselves, or is to_basic_gates() defined recursively?

@dlyongemallo
Copy link
Contributor Author

There are a few outstanding gates in qelib1.inc and stdgates.inc which haven't been included. I wanted to make sure that these are correct first, before I implement those.

@jvdwetering
Copy link
Collaborator

to_basic_gates should itself return a list of basic gates, so it should not have to be called recursively.

@dlyongemallo
Copy link
Contributor Author

I fixed the to_basic_gates for the newly added gates.

The only remaining gates which are in the OpenQASM standard libraries which aren't implemented yet are: rccx, rc3x, c3x, c3sqrtx, and c4x. I can add them later, or I can add them in this PR if you think they're necessary.

@jvdwetering
Copy link
Collaborator

I've never seen anyone use those gates, so I think it's fine if these are not implemented for now.

@jvdwetering
Copy link
Collaborator

Can this be merged?

@dlyongemallo
Copy link
Contributor Author

AFAIK, yes. Unless you have any comments or requests about the code.

@jvdwetering jvdwetering merged commit db42057 into zxcalc:master Oct 5, 2023
4 checks passed
@dlyongemallo
Copy link
Contributor Author

p.s. I just want to thank @jakelishman for his guidance in explaining the difference between how OpenQASM 2 and 3 represents the gates, which made this PR possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cp gate support for QASMParser
2 participants