From 6a5a3df2b679352c532248fb2093a2c593c3fa57 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 2 Oct 2024 15:43:41 -0400 Subject: [PATCH] Fix bug in QPY symengine payload version handling This commit fixes a bug that slipped into the sole fix that made up the now yanked 1.2.3 release, #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). --- qiskit/qpy/__init__.py | 6 +++--- qiskit/qpy/common.py | 2 +- qiskit/qpy/interface.py | 2 +- .../fix-qpy-parsing-123-75357c3709e35963.yaml | 10 +++++++++ ...qpy-symengine-compat-858970a9a1d6bc14.yaml | 6 +++--- test/qpy_compat/get_versions.py | 2 +- test/qpy_compat/run_tests.sh | 21 ++++++++++++++++++- 7 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/fix-qpy-parsing-123-75357c3709e35963.yaml diff --git a/qiskit/qpy/__init__.py b/qiskit/qpy/__init__.py index d6928a134a8..950b78fc421 100644 --- a/qiskit/qpy/__init__.py +++ b/qiskit/qpy/__init__.py @@ -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 @@ -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 diff --git a/qiskit/qpy/common.py b/qiskit/qpy/common.py index 6084f53a170..8d8a57b7404 100644 --- a/qiskit/qpy/common.py +++ b/qiskit/qpy/common.py @@ -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" ) diff --git a/qiskit/qpy/interface.py b/qiskit/qpy/interface.py index 827857ab2b0..2410e4b0a79 100644 --- a/qiskit/qpy/interface.py +++ b/qiskit/qpy/interface.py @@ -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. diff --git a/releasenotes/notes/fix-qpy-parsing-123-75357c3709e35963.yaml b/releasenotes/notes/fix-qpy-parsing-123-75357c3709e35963.yaml new file mode 100644 index 00000000000..d911a59879f --- /dev/null +++ b/releasenotes/notes/fix-qpy-parsing-123-75357c3709e35963.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixed an issue introduced in the now + `yanked `__ 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. diff --git a/releasenotes/notes/fix-qpy-symengine-compat-858970a9a1d6bc14.yaml b/releasenotes/notes/fix-qpy-symengine-compat-858970a9a1d6bc14.yaml index 359106de496..782ce85e0b1 100644 --- a/releasenotes/notes/fix-qpy-symengine-compat-858970a9a1d6bc14.yaml +++ b/releasenotes/notes/fix-qpy-symengine-compat-858970a9a1d6bc14.yaml @@ -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, @@ -25,7 +25,7 @@ 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 @@ -33,7 +33,7 @@ issues: - | 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, diff --git a/test/qpy_compat/get_versions.py b/test/qpy_compat/get_versions.py index a89e3b8508d..823a3a189db 100644 --- a/test/qpy_compat/get_versions.py +++ b/test/qpy_compat/get_versions.py @@ -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( diff --git a/test/qpy_compat/run_tests.sh b/test/qpy_compat/run_tests.sh index 437585bac58..979306a8378 100755 --- a/test/qpy_compat/run_tests.sh +++ b/test/qpy_compat/run_tests.sh @@ -26,7 +26,8 @@ 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" @@ -34,3 +35,21 @@ python -m venv "$qiskit_venv" 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