Qiskit / qiskit-qasm3-import

Importer from OpenQASM 3 to Qiskit's QuantumCircuit
https://qiskit.github.io/qiskit-qasm3-import
Apache License 2.0
15 stars 7 forks source link

Node of type "SubroutineDefinition" is not supported #8

Open ArfatSalman opened 1 year ago

ArfatSalman commented 1 year ago

Env

Code

from qiskit import QuantumCircuit, ClassicalRegister, QuantumRegister
from qiskit.circuit.library.standard_gates import *

qr = QuantumRegister(8, name="qr")
cr = ClassicalRegister(8, name="cr")
qc = QuantumCircuit(qr, cr, name="qc")
qc.append(RZGate(6.163759533339787), qargs=[qr[4]], cargs=[])

subcircuit = QuantumCircuit(qr, cr, name="subcircuit")

subcircuit.append(CZGate(), qargs=[qr[0], qr[2]], cargs=[])
qc.append(subcircuit, qargs=qr, cargs=cr)

qc.append(XGate(), qargs=[qr[2]], cargs=[])

from qiskit.qasm3 import loads, dumps

qc = loads(dumps(qc))
# qiskit.qasm3.exceptions.QASM3ImporterError: '3,0: node of type SubroutineDefinition is not supported'

I understand that some things are not yet supported by the Importer (See https://github.com/Qiskit/qiskit-terra/issues/9609) but is this a case of that? It seems like openqasm python lib does provide support for SubroutineDefinition (Link).

It is totally possible that this is planned for the future, in which case, feel free to ignore this issue. 😊

jlapeyre commented 1 year ago

(I answered this a few hours ago. But did not hit the green button, or it's in the wrong place, or.... I'll repeat it here.)

Please do open issues like this. I helps to understand the level of demand for features.

It seems like openqasm python lib does provide support

This is the openqasm reference parser. Its main purpose is to document the language and aid development. It is meant to ingest anything that is correct OQ3. (And because it doesn't do much semantic analysis, it also accepts some incorrect programs) It happens to be the most convenient thing to use for importing into Qiskit until a more robust solution is ready. But Qiskit can't represent all of OQ3. So the importer does not make use of everything that the parser gives it.

I don't know of plans to implement the feature you mention above. But on the face of it, it seems like a reasonable feature to add to the importer.

jlapeyre commented 1 year ago

@jakelishman what do you think? Does it make sense to support this? If so, in this version of the importer?

jakelishman commented 1 year ago

I think I originally made the decision to reject this because the Terra exporter's handling of subroutine outputs was really pretty questionable (if I remember correctly). The problems we'd face are that OQ3 subroutines have general parameter lists that have qubit arguments and arbitrary typed classical arguments all mixed together. Terra doesn't have much of a natural representation of those, especially functions with classical parameters, so I didn't want to add support for something that would be really half-baked at best.

That said, if we really did want to, it's not impossible to have some baseline of support for def subroutines that have only qubit/qubit[], bit/bit[] and angle inputs, and don't return anything. For prioritisation purposes, though, I'm not keen to commit to this - I think it's a fair amount of work that's quite tricky, and it's getting ahead of what Terra can naturally represent. Since we're also still interested in properly replacing this parser, I'm not super keen on extending it too much.

The reason we do want to support programs written with physical qubits is because we want to reach feature parity with what qiskit-ibm-provider (more correctly, IBM's internal QSS stack) can accept in the string-import form of dynamic circuits sooner rather than later.