-
Notifications
You must be signed in to change notification settings - Fork 98
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
Basic interop with Qiskit #1899
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the RE related parts. Looks good to me. Thanks @idavis !
Co-authored-by: Stefan J. Wernli <swernli@microsoft.com>
Co-authored-by: Stefan J. Wernli <swernli@microsoft.com>
Co-authored-by: Mine Starks <16928427+minestarks@users.noreply.github.com>
Co-authored-by: Stefan J. Wernli <swernli@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving with a few final comments -- I've tested it and read the code numerous times times, and I think it is certainly ready to go in and have any future improvements handled as future PRs.
Note that I did not review the .py
files outside of documentation and general sensibility, as I'm not deep on modern Python idioms or API standards, so it'd be best if you treat this approval as an approval of everything that isn't *.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Focused on the Python bits, not as much on the compiler internals. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved. I focused on the compiler/
folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor nitpick, not critical before merge but up to you if you want to address it. Excited to see this get in!
This PR adds the ability to resource estimate, run, and generate QIR from Qiskit
QuantumCircuit
s.There are three areas which we can break up the review into:
The QASM parser doesn't have usable semantic validation, so we have to do a lot of our own validation as we compile the QASM generated from the
QuantumCircuit
s.Closes #1074