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

Move OpenQASM 2 exporter to qiskit.qasm2 #10533

Merged
merged 8 commits into from
Sep 1, 2023

Conversation

jakelishman
Copy link
Member

Summary

This gets us into a position where the interface is similar between the qasm2, qasm3 and qpy modules (all of which do vaguely similar things at an abstract level), which are in turn similar to functionality in the Python standard libraries.

This commit simply moves the code from QuantumCircuit.qasm over to the new location. Some idiosyncrasies and additional bits of features that we do not intend to support long term (e.g. formatted output) are not brought over, but QuantumCircuit.qasm continues to perform those jobs until it is deprecated and removed.

Details and comments

Depends on #10532 because I copied across the new test from that as well - I didn't want that test to accidentally get lost later. The tests will fail on that one til that PR merges.

This gets us into a position where the interface is similar between the
`qasm2`, `qasm3` and `qpy` modules (all of which do vaguely similar
things at an abstract level), which are in turn similar to functionality
in the Python standard libraries.

This commit simply moves the code from `QuantumCircuit.qasm` over to the
new location.  Some idiosyncrasies and additional bits of features that
we do not intend to support long term (e.g. formatted output) are not
brought over, but `QuantumCircuit.qasm` continues to perform those
jobs until it is deprecated and removed.
@jakelishman jakelishman added Changelog: Deprecation Include in "Deprecated" section of changelog Changelog: New Feature Include in the "Added" section of the changelog mod: qasm2 Relating to OpenQASM 2 import or export labels Jul 31, 2023
@jakelishman jakelishman added this to the 0.45.0 milestone Jul 31, 2023
@jakelishman jakelishman requested a review from a team as a code owner July 31, 2023 11:41
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5717766407

  • 144 of 149 (96.64%) changed or added relevant lines in 3 files are covered.
  • 4 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.006%) to 85.916%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/qasm2/export.py 130 132 98.48%
qiskit/circuit/quantumcircuit.py 13 16 81.25%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 93.76%
crates/qasm2/src/lex.rs 1 91.65%
qiskit/extensions/quantum_initializer/squ.py 2 80.0%
Totals Coverage Status
Change from base Build 5717545484: 0.006%
Covered Lines: 73098
Relevant Lines: 85081

💛 - Coveralls

@classmethod
@property
@deprecate_func(
since="0.45.0", additional_msg="No alternative will be provided.", is_property=True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add something like handled by .qasm2.dumps, so no alternative is needed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunno - the use of this object is/was already done by internally QuantumCircuit.qasm, so if someone's using it outside that context, then they presumably also don't want to use the new qasm2.dumps.

@classmethod
@property
@deprecate_func(
since="0.45.0", additional_msg="No alternative will be provided.", is_property=True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general... some comments here and there.

Comment on lines +1604 to +1606
:func:`.qasm2.dump` and :func:`.qasm2.dumps`
The preferred entry points to the OpenQASM 2 export capabilities. These match the
interface for other serialisers in Qiskit.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just deprecate the .qasm method? Or that's done somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is just adding the alternative now, so QuantumCircuit.qasm isn't eligible for deprecation yet.

Comment on lines -1747 to +1632
file.write(out)
print(out, file=file)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this preferred? I always use the write way. Am I doing it wrong?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The print call here ensures that a terminating newline character is added. Since dumps returns an unterminated string (since that makes more sense for in-memory / interactive usage), I switched the file.write to print here so that the files would come out properly formatted without needing to string append \n manually or needing to make two Python-space write calls.

and exporting back to OpenQASM 2. The parsing components live in this module, while currently the
export capabilities are limited to being the :meth:`.QuantumCircuit.qasm` method.
Qiskit has support for interoperation with OpenQASM 2.0 programs, both :ref:`parsing into Qiskit
formats <qasm2-parse>` and :ref:`exporting back to OpenQASM 2 <qasm2-export>`.

.. note::

OpenQASM 2 is a simple language, and not suitable for general serialisation of Qiskit objects.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
OpenQASM 2 is a simple language, and not suitable for general serialisation of Qiskit objects.
OpenQASM 2 is a simple language, and not suitable for general serialisation of Qiskit circuits.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd done this deliberately because QPY also allows serialisation of ScheduleBlock, and sometimes the circuit structure is serialisable, but one or two of the contained instructions / expressions aren't. I can change it if you think that distinction is too subtle, though.


import pathlib

qiskit.qasm2.dump(qc, pathlib.Path.home() / "myfile.qasm")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didnt know that pathlib.Path.home() / "myfile.qasm" was possible. Nice one!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, pathlib is typically a much more convenient interface to working with filesystems than raw os.path, and almost everything that deals with path-like objects will accept T | os.PathLike[T] as the type (for T: str | bytes), of which pathlib.Path is one.

qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/qasm2/__init__.py Outdated Show resolved Hide resolved
qiskit/qasm2/export.py Outdated Show resolved Hide resolved
become confused.
deprecations:
- |
The little-used :class:`.QuantumCircuit` class data attributes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The little-used :class:`.QuantumCircuit` class data attributes
The :class:`.QuantumCircuit` class data attributes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to stress that these are highly unlikely to be in use, but can change it if you think it's much better not to.

@1ucian0 1ucian0 self-assigned this Sep 1, 2023
qiskit/qasm2/export.py Outdated Show resolved Hide resolved
Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
Copy link
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge it!

@1ucian0 1ucian0 added this pull request to the merge queue Sep 1, 2023
Merged via the queue into Qiskit:main with commit e1fdb72 Sep 1, 2023
13 checks passed
@jakelishman jakelishman deleted the qasm2/new-api branch October 11, 2023 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Deprecation Include in "Deprecated" section of changelog Changelog: New Feature Include in the "Added" section of the changelog mod: qasm2 Relating to OpenQASM 2 import or export
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants