Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Using includes with the QASM3 Exporter raises instead of including definitions #13061

Closed
idavis opened this issue Aug 30, 2024 · 7 comments · Fixed by #13148
Closed

Using includes with the QASM3 Exporter raises instead of including definitions #13061

idavis opened this issue Aug 30, 2024 · 7 comments · Fixed by #13148
Labels
bug Something isn't working mod: qasm3 Related to OpenQASM 3 import or export
Milestone

Comments

@idavis
Copy link

idavis commented Aug 30, 2024

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 idavis added the bug Something isn't working label Aug 30, 2024
@idavis
Copy link
Author

idavis commented Sep 12, 2024

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

@jakelishman
Copy link
Member

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?

@jakelishman jakelishman added this to the 1.2.2 milestone Sep 12, 2024
@jakelishman jakelishman added the mod: qasm3 Related to OpenQASM 3 import or export label Sep 12, 2024
@idavis
Copy link
Author

idavis commented Sep 12, 2024

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

@jakelishman
Copy link
Member

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
Copy link
Author

idavis commented Sep 13, 2024

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

@idavis
Copy link
Author

idavis commented Sep 13, 2024

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
Copy link
Member

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working mod: qasm3 Related to OpenQASM 3 import or export
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants