Skip to content

Commit

Permalink
Fix bug in QPY symengine payload version handling (Qiskit#13259)
Browse files Browse the repository at this point in the history
This commit fixes a bug that slipped into the sole fix that made up
the now yanked 1.2.3 release, Qiskit#13251. The fix logic had a typo/mistake
when evaluating the major version in the symengine payload that meant
if you were trying to load a QPY payload generated with a different
symengine release it would always error. This only got through CI because
there was no test coverage of the edge case we were trying to fix. This
commit addresses both issues. It first fixes the typo in the QPY parsing
(which is a 2 character change) and then updates the QPY compat tests to
explicitly test using multiple symengine versions with QPY to make sure
we're exercising this code path moving forward (and validating this
fix).
  • Loading branch information
mtreinish authored and ElePT committed Oct 3, 2024
1 parent 98a8f9d commit 8d1d5e2
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 10 deletions.
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"

"$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

0 comments on commit 8d1d5e2

Please sign in to comment.