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

Node of type "SubroutineDefinition" is not supported #8

Open
ArfatSalman opened this issue Feb 19, 2023 · 3 comments
Open

Node of type "SubroutineDefinition" is not supported #8

ArfatSalman opened this issue Feb 19, 2023 · 3 comments

Comments

@ArfatSalman
Copy link

Env

  • Qiskit Terra version: {'qiskit-terra': '0.23.1', 'qiskit-aer': '0.11.2', 'qiskit-ignis': None, 'qiskit-ibmq-provider': '0.20.0', 'qiskit': '0.41.0', 'qiskit-nature': None, 'qiskit-finance': None, 'qiskit-optimization': None, 'qiskit-machine-learning': None}
  • Python version: Python 3.9.12
  • Operating system: MacOS Ventura 13.1

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 Qiskit/qiskit#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
Copy link
Collaborator

jlapeyre commented Feb 19, 2023

(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
Copy link
Collaborator

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

@jakelishman
Copy link
Member

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.

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

No branches or pull requests

3 participants