Native OpenQASM 3 importer

[jakelishman]

Summary

This is a Qiskit-native version of an OpenQASM 3 parser and converter to QuantumCircuit, with the vast majority written in Rust.

At the moment, it exposes two entry points on qiskit.qasm3: loads_experimental and load_experimental, with their API documented.

Details and comments

This is hugely experimental right now, and this PR is currently missing a few critical features, some because of lack of support in the Rust code here, some because of lack of support on the parser side at https://github.com/Qiskit/openqasm3_parser.

I don't want to understate that above point: at the moment, measure isn't supported (but see Qiskit/openqasm3_parser#42 - it's being parsed, just there's a bug in passing it through to the semantic AST). Measure and barrier are now both supported, but (for example) if statements aren't.

I'm opening this PR in draft now to make it public, but there may be significant changes to the Rust internals, and obviously it will expand with new features from both sides. This PR currently includes a re-implementation of #11326 (ExperimentalWarning - since it was convenient to get it sorted immediately), and modifies qiskit.qasm3.dumps to use an OPENQASM 3.0; header (like #11418, except without the test modifications) because the parser at the moment doesn't like OPENQASM 3; (and we should be more accurate anyway). now rebased over main.

I haven't written the test suite yet.

As a brief example (and tbh, this shows much of the end-to-end supported feature set - it's not huge yet):

import numpy as np
import warnings

import qiskit.qasm2
import qiskit.qasm3
from qiskit.exceptions import ExperimentalWarning
from qiskit.circuit.library import RealAmplitudes

warnings.filterwarnings("ignore", category=ExperimentalWarning)

# This is a little more than 5k gates.
base = RealAmplitudes(27, reps=100, flatten=True)
base = base.assign_parameters(np.random.rand(base.num_parameters))
prog2 = qiskit.qasm2.dumps(base)
prog3 = qiskit.qasm3.dumps(base)

print("OpenQASM 2 time")
%timeit qiskit.qasm2.loads(prog2, custom_instructions=qiskit.qasm2.LEGACY_CUSTOM_INSTRUCTIONS)
print("Current OpenQASM 3 time")
%timeit qiskit.qasm3.loads(prog3)
print("New OpenQASM 3 time")
%timeit qiskit.qasm3.loads_experimental(prog3)

OpenQASM 2 time
29.1 ms ± 477 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
Current OpenQASM 3 time
1.9 s ± 19.5 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
New OpenQASM 3 time
58.8 ms ± 751 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)

[jakelishman]

Apologies - the Rust code isn't super well commented or structured at the moment. I will add more of that in due course. At the moment, in principle, everything happens in Rust, calling out to Python through PyO3 (so I didn’t need to bother defining a transport format) to construct a QuantumCircuit. The library files are:

• lib defines the Python entry points (loads)
• error at the moment just imports a Python-space exception
• circuit defines a few thin Rust-space wrappers around Python objects to inject a little bit more type safety into what I was doing
• expr is the constant-folder / expression evaluator (which doesn’t do much constant folding, but it does evaluate a few expressions)
• build is where the loop through the ASG happens, and that manages the construction of the QuantumCircuit

[jakelishman]

This dummy version of the standard include is because the base parser can't handle the fancy gate-annotations (gate cx a, b { ctrl @ x a, b; } and the like) that the canonical version of the file uses, so this is the same file with all the definitions stubbed out, just for import-parsing purposes.

[jlapeyre]

gate cx a, b { ctrl @ x a, b; } is now parsed correctly by version 0.0.7 of the base parser. (also in 0.0.6, but into a different structure.)

[jakelishman]

Ok cool thanks, I'll remove the dummy library if the regular stdgates.inc parses.

[coveralls]

Pull Request Test Coverage Report for Build 7734314938

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

• -208 of 903 (76.97%) changed or added relevant lines in 6 files are covered.
• 270 unchanged lines in 30 files lost coverage.
• Overall coverage decreased (-0.3%) to 89.327%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/qasm3/src/error.rs	0	1	0.0%
crates/qasm3/src/lib.rs	105	115	91.3%
crates/qasm3/src/circuit.rs	163	201	81.09%
crates/qasm3/src/expr.rs	203	282	71.99%
crates/qasm3/src/build.rs	211	291	72.51%

Files with Coverage Reduction	New Missed Lines	%
qiskit/circuit/library/standard_gates/rx.py	1	98.55%
qiskit/circuit/library/standard_gates/ry.py	1	98.51%
qiskit/circuit/library/standard_gates/rz.py	1	98.46%
qiskit/circuit/library/standard_gates/swap.py	1	98.08%
qiskit/circuit/library/standard_gates/u3.py	1	98.9%
qiskit/primitives/backend_estimator.py	1	96.14%
qiskit/transpiler/target.py	1	93.9%
qiskit/circuit/library/standard_gates/p.py	2	98.0%
qiskit/circuit/library/standard_gates/u.py	2	97.78%
qiskit/primitives/containers/estimator_pub.py	2	97.53%

Totals
Change from base Build 7729918317:	-0.3%
Covered Lines:	60087
Relevant Lines:	67266

---

💛 - Coveralls

[jakelishman]

Whoopsie, that last commit uses a Rust 1.75 feature: return-position impl Trait in a trait function!

[jlapeyre]

NOTE: The following PR has been closed, not merged.
See jakelishman#22

[qiskit-bot]

One or more of the the following people are requested to review this:

• @Eric-Arellano
• @Qiskit/terra-core
• @kevinhartman
• @mtreinish

[jakelishman]

Long overdue removing from draft, sorry about that.

[jakelishman]

I've pushed up the formatting fix (I really ought to sort out my editor - I'm so used to having autoformat setup for Python that I always forget I don't have Rust set to format on write), but I've not reverted the dummy library directory for stdgates.inc because of Qiskit/openqasm3_parser#88. I don't think that's critical - it's not really a problem to ship with the dummy data directory.

[jlapeyre]

This looks ready to go to me. @mtreinish ?

[mtreinish]

Thanks for the quick update. One thing I missed in my earlier review is that for an experimental api the requirements are it's clearly documented and it raises a warning on use. I just noticed the documentation side of this is missing in the function docstrings. The other missing thing is the release note, I think even as an experimental feature this definitely warrants a feature note. Once those are fixed I think this is good to go.

[jlapeyre]

Does this pin the version at 0.0.7? In principle we could make backward compatible bug fixes in 0.0.8. There are certainly some bugs that would affect this importer.

[jlapeyre]

It looks like we might want to release 0.1.0 of the parser in order to allow fixes:
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio

[jlapeyre]

Then oq3_semantics = "0.1.0" would be compatible with 0.1.1, etc.

[jakelishman]

Yes, this dependency is equivalent to >=0.0.7,<0.0.8. Cargo treats leading zeros specially - it basically treats the leftmost non-zero as the semantic-version "major". For how we're updating the base library at the moment, I think that's actually fairly accurate (and not a problem on either side).

That said, this doesn't really affect anything, because we build our Rust crates into cdylib Python extensions that we treat give-or-take like fixed binary objects - one of those ways is that we pin our dependencies with the lockfile, so only precise lockfile updates affect the build environment, and (for the most part) are made separately to Rust code changes. In this case, large tracts of the lockfile are updated because there's so many new dependencies getting pulled in.

[jlapeyre]

In any case, I just released openqasm3_parser 0.1.0 ., This is the same as 0.0.7. Just changed the version (and reduced the sleep between publishing crates to 60 s) : https://crates.io/crates/oq3_semantics

I'm not sure I follow everything you said about versions. But if I understand correctly, a particular version of Qiskit includes a fixed version of every file, including the Cargo.lock files. And these
fix precise versions of Rust dependencies of Qiksit.

[jakelishman]

I've done a pass at the release note in c64c8c1, though I expect we might expand it or chop/change it as we write the 1.0 prelude next week.

I also threw in an extra test of gate broadcasting, which Coveralls pointed out I'd missed, in 6712183.

[jakelishman]

Just thinking: perhaps the largest thing that makes me nervous about this is the problems around negative float literals. If we're able to quickly teach the backend lexer to lex float (and int) literals as -?<whatever-it-is> and rely on maximal-munch to disambiguate (as we already are in int vs float, really), I'd feel a lot better.

[mtreinish]

LGTM now thanks for the update. Just one quick inline comment on some of the new docs.

[mtreinish]

Just thinking: perhaps the largest thing that makes me nervous about this is the problems around negative float literals. If we're able to quickly teach the backend lexer to lex float (and int) literals as -?<whatever-it-is> and rely on maximal-munch to disambiguate (as we already are in int vs float, really), I'd feel a lot better.

I think it's fine to do this between rc1 and the final release. This api endpoint is explicitly experimental and actively warns as such, so changes in behavior seem fair game at any point. But also since it wouldn't be changing any public interface I'd view this as more of a bugfix than anything else.

[jakelishman]

Yeah, that sounds reasonable to me.

[jlapeyre]

perhaps the largest thing that makes me nervous about this is the problems around negative float literals

I spent a lot of time on the weekend and today on gate modifiers, which turns out not to be important for the release! ... Maybe we can call this the fog of major version release. Anyway, I might be able to fix unary minus pretty quickly.

[jakelishman]

John: sorry about that. My aim (certainly in the later half) was less to support modifiers end-to-end, but more to allow us to drop the dummy include file for stdgates.inc, because that includes modifiers. The gphase thing made that tricky, though.

For floats: personally I'd rather handle that in the lexer rather than a unary minus op - it means that we don't have to handle constant-folding the unary minus, which means quite a lot of work on this side because there's no real constant-folding machinery. It's quite common to lex the minus sign as part of the literal. Ofc in the general case unary minus still needs to work (if not only for things like - 1.0 which should lex as two separate tokens), but -1.0 can easily lex to a float literal, then it'll all just work throughout with no changes anywhere else I think.

edit: That above paragraph makes no sense and hallucinates lexer theory, and I think I'm losing my mind.

[jlapeyre]

personally I'd rather handle that in the lexer rather than a unary minus op

Yes I agree that seems like the easiest way to handle floats.

[jlapeyre]

I notice FWIW that the reference lexer for OQ3 does not lex the minus sign as part of the float. There must be some reason people sometimes prefer to lex - always as a token by itself. I don't see an advantage really... except that you don't have to account for whitespace in the float literal because its part of lexing all tokens. Also, it's possible to lex - 1... (with whitespace) as a single token.

[jakelishman]

Thanks very much for that!

I'm really sorry - you pointing out we don't do it in the OQ3 ANTLR lexer made me realise why we don't actually lex like that - it'll produce false parse failures for a-1.2, for example, which will lex as an identifier followed by a float, which would appear to be an invalid program.

I don't even know where I got the idea that that was common to do lexer side, I'm sorry.

Do you think it's best to stick with this for one release, or switch it back?

[jlapeyre]

I don't even know where I got the idea that that was common to do lexer side,

Not a problem. This took only a few minutes. And it's useful to understand the issue.

Qiskit/openqasm3_parser#90 fails to parse a-1.2 correctly in exactly this way. I don't have tests in place to detect this. Writing tests is laborious and clumsy. The r-a code had a facility to extract tests from comments. I never got around to transferring it to the oq3 parser.

Do you think it's best to stick with this for one release, or switch it back?

If you mean Qiskit/openqasm3_parser#90, we have to revert. Before this commit,
rx(a-1.2) q; is parsed to ASG correctly. Afterwards, it is not. Again, we lack tests. It is relatively easy for me to add tests that simply count semantic errors. This is better than nothing.

I think correcting this between the lexer and the ASG will not be difficult.