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

Fix bug in QPY symengine payload version handling #13259

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
6 changes: 3 additions & 3 deletions qiskit/qpy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@

.. note::

With versions of Qiskit before 1.2.3, the ``use_symengine=True`` argument to :func:`.qpy.dump`
With versions of Qiskit before 1.2.4, the ``use_symengine=True`` argument to :func:`.qpy.dump`
could cause problems with backwards compatibility if there were :class:`.ParameterExpression`
objects to serialize. In particular:

* When the loading version of Qiskit is 1.2.3 or greater, QPY files generated with any version
* When the loading version of Qiskit is 1.2.4 or greater, QPY files generated with any version
of Qiskit >= 0.46.0 can be loaded. If a version of Qiskit between 0.45.0 and 0.45.3 was used
to generate the files, and the non-default argument ``use_symengine=True`` was given to
:func:`.qpy.dump`, the file can only be read if the version of ``symengine`` used in the
Expand All @@ -134,7 +134,7 @@
used in the generating environment.

To recover a QPY file that fails with ``symengine`` version-related errors during a call to
:func:`.qpy.load`, first attempt to use Qiskit >= 1.2.3 to load the file. If this still fails,
:func:`.qpy.load`, first attempt to use Qiskit >= 1.2.4 to load the file. If this still fails,
it is likely because Qiskit 0.45.x was used to generate the file with ``use_symengine=True``.
In this case, use Qiskit 0.45.3 with ``symengine==0.9.2`` to load the file, and then re-export
it to QPY setting ``use_symengine=False``. The resulting file can then be loaded by any later
Expand Down
2 changes: 1 addition & 1 deletion qiskit/qpy/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ def load_symengine_payload(payload: bytes) -> symengine.Expr:
major = payload[2]
minor = payload[3]
if int(symengine_version[1]) != minor:
if major != "0":
if major != 0:
raise exceptions.QpyError(
"Qiskit doesn't support loading a symengine payload generated with symengine >= 1.0"
)
Expand Down
2 changes: 1 addition & 1 deletion qiskit/qpy/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def dump(
If serializing a :class:`.QuantumCircuit` or :class:`.ScheduleBlock` that contain
:class:`.ParameterExpression` objects with ``version`` set low with the intent to
load the payload using a historical release of Qiskit, it is safest to set the
``use_symengine`` flag to ``False``. Versions of Qiskit prior to 1.2.3 cannot load
``use_symengine`` flag to ``False``. Versions of Qiskit prior to 1.2.4 cannot load
QPY files containing ``symengine``-serialized :class:`.ParameterExpression` objects
unless the version of ``symengine`` used between the loading and generating
environments matches.
Expand Down
10 changes: 10 additions & 0 deletions releasenotes/notes/fix-qpy-parsing-123-75357c3709e35963.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
fixes:
- |
Fixed an issue introduced in the now
`yanked <https://peps.python.org/pep-0592/>`__ 1.2.3 bugfix release that
would cause an exception with the error message "Qiskit doesn't support
loading a symengine payload generated with symengine >= 1.0" to be raised
whenever loading a QPY file that was generated with a different symengine
version from the version installed by the loading. This issue could only
occur in 1.2.3.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fixes:
work.
issues:
- |
Versions of Qiskit before 1.2.3 will not be able to load QPY files dumped
Versions of Qiskit before 1.2.4 will not be able to load QPY files dumped
using :func:`.qpy.dump`, even with ``version`` set appropriately, if:

* there are unbound :class:`.ParameterExpression`\ s in the QPY file,
Expand All @@ -25,15 +25,15 @@ issues:
environments are not within the same minor version.

This applies regardless of the version of Qiskit used in the generation (at
least up to Qiskit 1.2.3 inclusive).
least up to Qiskit 1.2.4 inclusive).

If you want to maximize compatibility with older versions of Qiskit, you
should set ``use_symengine=False``. Newer versions of Qiskit should not
require this.
- |
QPY files from the Qiskit 0.45 series can, under a very specific and unlikely
set of circumstances, fail to load with any newer version of Qiskit,
including Qiskit 1.2.3. The criteria are:
including Qiskit 1.2.4. The criteria are:

* the :class:`.QuantumCircuit` or :class:`.ScheduleBlock` to be dumped
contained unbound :class:`.ParameterExpression` objects,
Expand Down
2 changes: 1 addition & 1 deletion test/qpy_compat/get_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def available_versions_for_package(package, min_version=None, max_version=None):
if not any(
tag in supported_tags
for release in payload
if release["packagetype"] == "bdist_wheel"
if release["packagetype"] == "bdist_wheel" and not release["yanked"]
for tag in tags_from_wheel_name(release["filename"])
):
print(
Expand Down
21 changes: 20 additions & 1 deletion test/qpy_compat/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,30 @@ qiskit_venv="$(pwd -P)/qiskit_venv"
qiskit_python="$qiskit_venv/bin/python"
python -m venv "$qiskit_venv"
# `packaging` is needed for the `get_versions.py` script.
"$qiskit_venv/bin/pip" install -c "$our_dir/../../constraints.txt" "$our_dir/../.." packaging
# symengine is pinned to 0.13 to explicitly test the migration path reusing the venv
"$qiskit_venv/bin/pip" install -c "$our_dir/../../constraints.txt" "$our_dir/../.." packaging "symengine~=0.13"
Comment on lines +29 to +30
Copy link
Member

Choose a reason for hiding this comment

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

This isn't actually a pin on symengine 0.13 - it's equivalent to symengine>=0.13,==0.*, so it'd allow 0.14, but I think that's intended, given the comment lower in the file about needing an update when symengine 0.14 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.

Oh I actually meant to remove this. I just checked it's in my local diff but I didn't add that to the commit. I started down the path of thinking about pinning here, but figured it'd be better to handle it explicitly at that future date than protect against it in the test setup preemptively. Especially since we have the cap in the requirements file. I can remove this if you'd prefer.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's ok as written - it's good to be explicit that it's specifically something newer than the 0.11 series, which this pin does.

It's just the comment that's a bit misleading, but I strongly suspect we're going to need to re-do this file a little bit as part of making the cache keys for the QPY jobs more reliable - things like generation script requirements will need to be separated off to become part of the cache key - and we'll be able to expand the inter-symengine-version testing once that's fixed as well, because the jobs will run faster.


"$qiskit_python" "$our_dir/get_versions.py" | parallel --colsep=" " bash "$our_dir/process_version.sh" -p "$qiskit_python"

# Test dev compatibility
dev_version="$("$qiskit_python" -c 'import qiskit; print(qiskit.__version__)')"
"$qiskit_python" "$our_dir/test_qpy.py" generate --version="$dev_version"
"$qiskit_python" "$our_dir/test_qpy.py" load --version="$dev_version"

# Test dev compatibility with different symengine versions across 0.11 and 0.13
#
# NOTE: When symengine >= 0.14.0 is released we will need to modify this to build an explicit
# symengine 0.13.0 venv instead of reusing $qiskit_venv.
#
symengine_11_venv="$(pwd -P)/qiskit_symengine_11_venv"
symengine_11_python="$symengine_11_venv/bin/python"
python -m venv "$symengine_11_venv"
"$symengine_11_venv/bin/pip" install -c "$our_dir/../../constraints.txt" "$our_dir/../.." "symengine==0.11.0"
# Load symengine 0.13.0 generated payload with symengine 0.11
"$symengine_11_python" "$our_dir/test_qpy.py" load --version="$dev_version"
# Load symengine 0.11.0 generated payload with symengine 0.13.0
mkdir symengine_11_qpy_files
pushd symengine_11_qpy_files
"$symengine_11_python" "$our_dir/test_qpy.py" generate --version="$dev_version"
"$qiskit_python" "$our_dir/test_qpy.py" load --version="$dev_version"
popd
Loading