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

Emit a descriptive error when the QPY version is too new #11094

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions qiskit/qpy/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,11 @@ def load(
file_obj.read(formats.FILE_HEADER_SIZE),
)
)
if data.qpy_version > common.QPY_VERSION:
raise QiskitError(
f"The QPY format version being read, {data.qpy_version}, isn't supported by "
"this Qiskit version. Please upgrade your version of Qiskit to load this QPY payload"
)
if data.preface.decode(common.ENCODE) != "QISKIT":
raise QiskitError("Input file is not a valid QPY file")
version_match = VERSION_PATTERN_REGEX.search(__version__)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
fixes:
- |
Fixed an issue with :func:`.qpy.load` when attempting to load a QPY format
version that is not supported by this version of Qiskit it will now display
a descriptive error message. Previously, it would raise an internal error
because of the incompatibility between the formats which was difficult to
debug. If the QPY format verison is not supported that indicates the Qiskit
version will need to be upgraded to read the QPY payload.
19 changes: 18 additions & 1 deletion test/python/qpy/test_circuit_load_from_qpy.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@
"""Test cases for the schedule block qpy loading and saving."""

import io
import struct

from ddt import ddt, data

from qiskit.circuit import QuantumCircuit, QuantumRegister, Qubit
from qiskit.providers.fake_provider import FakeHanoi, FakeSherbrooke
from qiskit.qpy import dump, load
from qiskit.exceptions import QiskitError
from qiskit.qpy import dump, load, formats
from qiskit.qpy.common import QPY_VERSION
from qiskit.test import QiskitTestCase
from qiskit.transpiler import PassManager, TranspileLayout
from qiskit.transpiler import passes
Expand Down Expand Up @@ -71,6 +74,20 @@ def test_rzx_calibration_echo(self, angle):
self.assert_roundtrip_equal(rzx_qc)


class TestVersions(QpyCircuitTestCase):
"""Test version handling in qpy."""

def test_invalid_qpy_version(self):
"""Test a descriptive exception is raised if QPY version is too new."""
with io.BytesIO() as buf:
buf.write(
struct.pack(formats.FILE_HEADER_PACK, b"QISKIT", QPY_VERSION + 4, 42, 42, 1, 2)
)
buf.seek(0)
with self.assertRaisesRegex(QiskitError, str(QPY_VERSION + 4)):
load(buf)


@ddt
class TestLayout(QpyCircuitTestCase):
"""Test circuit serialization for layout preservation."""
Expand Down
2 changes: 2 additions & 0 deletions test/qpy_compat/process_version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ version=$1
parts=( ${version//./ } )
if [[ ${parts[1]} -lt 18 ]] ; then
exit 0
elif [[ ${parts[1]} -gt 25 ]] ; then
exit 0
Comment on lines +21 to +22
Copy link
Member

Choose a reason for hiding this comment

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

I'm fine to merge this directly to the stable/0.25 branch, since that branch is about to be staled, but for the main version, it'd be good to do this more dynamically so it doesn't need to be hardcoded during the release process.

Copy link
Member

Choose a reason for hiding this comment

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

Also technically this whole set of checkers will stop working right with the release of Qiskit 1.0, but again, that's a separate problem.

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, I've started to draft a PR to fix this locally. I'll push up a proper fix to main (and for stable/0.45 shortly). I did it this way for stable/0.25 in the interest of unblocking the 0.25.3 release and because we'll stop supporting the branch after 0.45.0 final is released.

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 pushed up the PR for this here: #11101

fi

if [[ ! -d qpy_$version ]] ; then
Expand Down