Using includes with the QASM3 Exporter raises instead of including definitions

idavis commented 2 months ago

Environment

Qiskit version: 1.2
Python version: 3.11
Operating system: Sonoma 14.6.1 (23G93) aarch64

What is happening?

When custom includes are passed into the qiskit/qasm3/exporter::Exporter.__init__ function, the export of QASM will always throw an error when the dump function is called via the QASM3Builder. The docs say:

includes: the filenames that should be emitted as includes.  These files will be parsed
    for gates, and any objects dumped from this exporter will use those definitions
    where possible.

This worked fine in Qiskit 1.1.x but 1.2 changed the behavior such that any includes passed in that aren't exactly stdgates.inc will raise QASM3ExporterError. This also removed the GlobalNamespace type breaking consumers.

In 1.1.x the definitions from the custom includes were treated as opaque assuming any missing definitions would have been provided in the basis gates passed into the Exporter.

How can we reproduce the issue?

from qiskit import QuantumCircuit
from qiskit.qasm3 import Exporter, QASM3ExporterError

try:
    qc = QuantumCircuit(1, 1)
    includes = ("stdgates.inc", "custom_gates.inc")
    Exporter(includes=includes).dumps(qc)
except QASM3ExporterError as ex:
    print(ex)

"Unknown OpenQASM 3 include file: 'custom_gates.inc'"

What should happen?

The documentation indicates that the includes will be parsed for gate definitions. The docs have not changed since 1.1.x so the docs have never matched the implementation. I think there are two primary paths which can be taken:

Restore the old functionality and update the documentation. This would allow custom gate definitions in QASM files.
- The basis_gates can be used to define opaque gate names which would be found in the external QASM files.
Update the Exporter/QASM3Builder to match the docs. This will require updating the crates/qasm3/build.rs as it currently noops gate definitions when parsing the ASG.
- How would this reconcile parsed definitions vs the basis_gates parameter?

These solutions would not include any files included from the supplied includes list. Instead, the supplied includes are listed at the top of the generated QASM file as was done in 1.1.x.

Any suggestions?

No response

idavis commented 2 months ago

@jakelishman Do you know who might be able to look at this issue?

jakelishman commented 2 months ago

Ah, raising the error on unknown imports was a mistake - I didn't think about how you could make it work by using basis_gates, and primarily was trying to fix the exporter from silently accepting something it actually ignored (the violated documentation you posted). That can be fixed in a patch release, sorry.

GlobalNamespace was never part of the public API (it was an internal detail of the exporter, and not a great one at that), so I'm not sure how removing it broke consumers - how were you using it?

idavis commented 2 months ago

I was using it to get the authoritative stdgates defined for QASM export. I worked around it by creating my own list.

jakelishman commented 2 months ago

I've made #13148, which should fix the bad behaviour around the includes argument - sorry about that, it was a clear mistake in #12776 to newly raise an error.

Fwiw, we do have a public-interface version of what's in stdgates.inc, at least as of spec version 3.0, in a tuple at qiskit.qasm3.STDGATES_INC_GATES: https://docs.quantum.ibm.com/api/qiskit/qasm3#qiskitqasm3stdgates_inc_gates. It's actually intended for the importer, but since they're both working off the OQ3 language spec, they're the same. The fully normative version of what stdgates.inc should be is documented by the OQ3 project: https://openqasm.com/language/standard_library.html#standard-library.

idavis commented 2 months ago

I'll try to use STDGATES_INC_GATES. Thank you very much!

idavis commented 2 months ago

Not sure if I'm missing something, but I can't seem to access the name on the CustomGate items.

from qiskit.qasm3 import STDGATES_INC_GATES
gate_names = [gate.name for gate in STDGATES_INC_GATES]
gate_names = [gate.name() for gate in STDGATES_INC_GATES]

Both give the error: 'qiskit._accelerate.qasm3.CustomGate' object has no attribute 'name'

jakelishman commented 2 months ago

Oh sorry, that's my fault - I didn't actually check that the data of a CustomGate is accessible in Python space after creation. We can make it accessible from Python space as a feature request.

Qiskit / qiskit