OpenQASM returned by convert_to_openqasm includes opaque gates.

[hodgestar]

Describe the bug

PyGSTI 0.9.12 introduce a new opaque delay(t) q; gate definition into the OpenQASM 2 produced by convert_to_openqasm in commit 4b740b3f1cd67ce6f0aab37fc77eb26b9699fe48. This defines an unspecified opaque gate named delay making the meaning of the returned OpenQASM program undefined.

Could this be replaced with a defined OpenQASM 2 construct?

It's not entirely clear to me why the old id gate was there either. If we want to ensure that all gates are used in portion of the output program, wouldn't a barrier on all the qubits suffice?

Perhaps it is also time to add an OpenQASM 3 exporter? If that's desired I could make an attempt and create a merge request.

Link to OpenQASM 2 spec so that I don't have to search for it again later.

To Reproduce

Run convert_to_openqasm.

Expected behavior

The returned OpenQASM program does not contain undefined gates.

Environment (please complete the following information):

• pyGSTi version: v0.9.12

[sserita]

Hi @hodgestar, thanks for the feedback!

I am however moving this from a bug to a discussion for the following reason: This was an explicit decision based on the deprecation of the id instruction from Qiskit a while back. The deprecation warning states to use the delay instruction instead; unfortunately, delay is not defined in qelib1.inc. The easiest way to solve this is just to define delay as an opaque instruction, which is then parsed into the Delay() operation by Qiskit's QASM2 parser (see https://docs.quantum.ibm.com/api/qiskit/qasm2#legacy-compatibility).

I will admit that it is an unsatisfactory solution, and is due to an inherent mismatch between Qiskit's QuantumCircuit class and OpenQASM 2. I would potentially be in favor of moving to QASM 3, where the proper way to do this is with delays and stretches - my only concern would be maintaining backwards compatibility with pyGSTi's current qasm output, which is used in some places for serialization, e.g. IBMQExperiment objects.

We'd certainly welcome a PR if you are interested in developing a QASM3 transpiler instead. You would need to sign a CLA first - email me at sserita@sandia.gov for a copy. Otherwise, a QASM3 transpiler will likely be on our to-do list at some point, but likely not for several months.

[hodgestar]

@sserita Thanks for the quick reply. Very happy for this to be a discussion instead of a bug.

For adding OpenQASM 3 support, my thought was to just add a convert_to_openqasm3 method. The convert_to_openqasm method could then be deprecated at some point down the line. Alternatively, one could add a version parameter to convert_to_openqasm. Somehow the separate method seems nicer to me, but I'd be happy to write it either way.

I'm still curious about why these zero delays or id gates are needed here.

[sserita]

Re: why they are needed: That's a fair question. It's almost certainly for some legacy reason - I will check with other team members and see if we remember why these were added and if we can remove them. That way, we could potentially fix this issue for you in the short-term, and spawn off the QASM3 converter as a new feature.

I can see both methods working, but I'm inclined to agree that a separate function seems a little cleaner. That way, we can just use a deprecated decorator on the old function whenever we want to remove it (which may not be quite yet, as there is still a lot of QASM2 floating around).

We would still need a Contributor License Agreement (CLA) from you before we could accept a PR - email me at sserita@sandia.gov to get that process started.

[hodgestar]

@sserita Any luck finding out why these strange operations are needed?

Once I know the answer I can make an OpenQASM 3 implementation.

[sserita]

Hey @hodgestar, there's now a flag to remove those delay instructions. You can pass include_delay_on_idle=False into convert_to_openqasm and it should drop the delays and be a valid OpenQASM. It's currently just in bugfix-release-0.9.12.1 branch, but that release will be out shortly.

I'm going to close this out if that's acceptable, and we can create a new issue whenever we start working on the OpenQASM 3.0 emitter.

[hodgestar]

Thanks @sserita. I was waiting on hearing what the delays were for before starting an OpenQASM 3 PR, but I'll take the new flag as meaning that the delays for zero time are probably not necessary.

Happy with this issue being closed. I'll open a PR for the OpenQASM 3 variant when I get there.

[sserita]

Yep that sounds good! In OpenQASM 3, we actually have delay as a valid instruction (and stretches to deal with variable time of the other gates so we don't have to hardcode a 0), but I can handle that logic when we get there if that's something that doesn't interest you in implementing. 😅

[hodgestar]

I'll figure it out -- you can comment on the PR if I get it wrong. :)