From 6ccb14ff07c309f9bb53101b08078df896f99636 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Thu, 3 Oct 2024 18:30:21 +0100 Subject: [PATCH] Move QPY tests to GitHub Actions and increase inter-symengine tests This commit has two major goals: - fix the caching of the QPY files for both the `main` and `stable/*` branches - increase the number of compatibility tests between the different symengine versions that might be involved in the generation and loading of the QPY files. Achieving both of these goals also means that it is sensible to move the job to GitHub Actions at the same time, since it will put more pressure on the Azure machine concurrency we use. Caching ------- The previous QPY tests attempted to cache the generated files for each historical version of Qiskit, but this was unreliable. The cache never seemed to hit on backport branches, which was a huge slowdown in the critical path to getting releases out. The cache restore keys were also a bit lax, meaning that we might accidentally have invalidated files in the cache by changing what we wanted to test, but the restore keys wouldn't have changed. The cache files would fail to restore as a side-effect of ed79d42 (gh-11526); QPY was moved to be on the tail end of the lint run, rather than in a test run. This meant that it was no longer run as part of the push event when updating `main` or one of the `stable/*` branches. In Azure (and GitHub Actions), the "cache" action accesses a _scoped_ cache, not a universal one for the repository [^1][^2]. Approximately, base branches each have their own scope, and PR events open a new scope that is a child of the target branch, the default branch, and the source branch, if appropriate. A cache task can read from any of its parent scopes, but write events go to the most local scope. This means that we haven't been writing to long-standing caches for some time now. PRs would typically miss the cache on the first attempt, hit their cache for updates, then miss again once entering the merge queue. The fix for this is to run the QPY job on branch-update events as well. The post-job cache action will then write out to a reachable cache for all following events. Cross-symengine tests --------------------- We previously were just running a single test with differing versions of symengine between the loading and generation of the QPY files. This refactors the QPY `run_tests.sh` script to run a full pairwise matrix of compatibility tests, to increase the coverage. [^1]: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/caching-dependencies-to-speed-up-workflows#restrictions-for-accessing-a-cache [^2]: https://learn.microsoft.com/en-us/azure/devops/pipelines/release/caching?view=azure-devops#cache-isolation-and-security --- ...docs_qpy-linux.yml => lint_docs-linux.yml} | 25 +----- .github/workflows/qpy.yml | 39 +++++++++ azure-pipelines.yml | 4 +- requirements.txt | 3 + test/qpy_compat/process_version.sh | 6 +- test/qpy_compat/run_tests.sh | 80 ++++++++++++++----- 6 files changed, 108 insertions(+), 49 deletions(-) rename .azure/{lint_docs_qpy-linux.yml => lint_docs-linux.yml} (61%) create mode 100644 .github/workflows/qpy.yml diff --git a/.azure/lint_docs_qpy-linux.yml b/.azure/lint_docs-linux.yml similarity index 61% rename from .azure/lint_docs_qpy-linux.yml rename to .azure/lint_docs-linux.yml index c08ea702ede5..b9e0c37892df 100644 --- a/.azure/lint_docs_qpy-linux.yml +++ b/.azure/lint_docs-linux.yml @@ -4,8 +4,8 @@ parameters: displayName: "Version of Python to use" jobs: - - job: 'Lint_Docs_QPY' - displayName: 'Lint, documentation and QPY' + - job: 'Lint_Docs' + displayName: 'Lint and documentation' pool: {vmImage: 'ubuntu-latest'} variables: @@ -43,24 +43,3 @@ jobs: artifactName: 'html_docs' Parallel: true ParallelCount: 8 - - - task: Cache@2 - inputs: - key: 'qpy | test/qpy_compat/test_qpy.py | "$(Build.BuildNumber)"' - restoreKeys: | - qpy | test/qpy_compat/test_qpy.py - path: qpy_files - displayName: Cache old QPY files - - - bash: | - set -e - # Reuse the docs environment to avoid needing to rebuild another - # version of Qiskit. - source .tox/docs/bin/activate - mv qpy_files/* test/qpy_compat || : - pushd test/qpy_compat - ./run_tests.sh - popd - mkdir -p qpy_files - mv test/qpy_compat/qpy_* qpy_files/. - displayName: 'Run QPY backwards compat tests' diff --git a/.github/workflows/qpy.yml b/.github/workflows/qpy.yml new file mode 100644 index 000000000000..2c127add907a --- /dev/null +++ b/.github/workflows/qpy.yml @@ -0,0 +1,39 @@ +name: QPY + +on: + push: + branches: + - 'main' + - 'stable/*' + pull_request: + merge_group: +concurrency: + group: ${{ github.repository }}-${{ github.ref }}-${{ github.head_ref }}-${{ github.workflow }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + backward_compat: + if: github.repository_owner == 'Qiskit' + name: Backwards compatibility + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.9' + + - uses: dtolnay/rust-toolchain@stable + + - uses: actions/cache@v4 + with: + path: test/qpy_compat/qpy_cache + # The hashing is this key can be too eager to invalidate the cache, + # but since we risk the QPY tests failing to update if they're not in + # sync, it's better safe than sorry. + key: qpy-${{ hashFiles('test/qpy_compat/**') }} + + - name: Run QPY backwards compatibility tests + working-directory: test/qpy_compat + run: ./run_tests.sh diff --git a/azure-pipelines.yml b/azure-pipelines.yml index d182ba32f1b3..94ed7d1fad7f 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -144,7 +144,7 @@ stages: - stage: "Lint_Docs_Prelim_Tests" displayName: "Preliminary tests" jobs: - - template: ".azure/lint_docs_qpy-linux.yml" + - template: ".azure/lint_docs-linux.yml" parameters: pythonVersion: ${{ parameters.minimumPythonVersion }} @@ -208,7 +208,7 @@ stages: - stage: "Merge_Queue" displayName: "Merge queue" jobs: - - template: ".azure/lint_docs_qpy-linux.yml" + - template: ".azure/lint_docs-linux.yml" parameters: pythonVersion: ${{ parameters.minimumPythonVersion }} diff --git a/requirements.txt b/requirements.txt index 4c13eb6dc60a..6eb5902ae9fd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,4 +6,7 @@ dill>=0.3 python-dateutil>=2.8.0 stevedore>=3.0.0 typing-extensions + +# If updating the version range here, consider updating 'test/qpy_compat/run_tests.sh' to update the +# list of symengine dependencies used in the cross-version tests. symengine>=0.11,<0.14 diff --git a/test/qpy_compat/process_version.sh b/test/qpy_compat/process_version.sh index c56c44f554bb..01998e01667f 100755 --- a/test/qpy_compat/process_version.sh +++ b/test/qpy_compat/process_version.sh @@ -42,14 +42,14 @@ package="$1" version="$2" our_dir="$(realpath -- "$(dirname -- "${BASH_SOURCE[0]}")")" -cache_dir="$(pwd -P)/qpy_$version" -venv_dir="$(pwd -P)/${version}" +cache_dir="$(pwd -P)/qpy_cache/$version" +venv_dir="$(pwd -P)/venvs/$package-$version" if [[ ! -d $cache_dir ]] ; then echo "Building venv for $package==$version" "$python" -m venv "$venv_dir" "$venv_dir/bin/pip" install -c "${our_dir}/qpy_test_constraints.txt" "${package}==${version}" - mkdir "$cache_dir" + mkdir -p "$cache_dir" pushd "$cache_dir" echo "Generating QPY files with $package==$version" "$venv_dir/bin/python" "${our_dir}/test_qpy.py" generate --version="$version" diff --git a/test/qpy_compat/run_tests.sh b/test/qpy_compat/run_tests.sh index 979306a83784..a6ff67b14a94 100755 --- a/test/qpy_compat/run_tests.sh +++ b/test/qpy_compat/run_tests.sh @@ -14,6 +14,7 @@ set -e set -o pipefail set -x +shopt -s nullglob # Set fixed hash seed to ensure set orders are identical between saving and # loading. @@ -21,35 +22,72 @@ export PYTHONHASHSEED=$(python -S -c "import random; print(random.randint(1, 429 echo "PYTHONHASHSEED=$PYTHONHASHSEED" our_dir="$(realpath -- "$(dirname -- "${BASH_SOURCE[0]}")")" +repo_root="$(realpath -- "$our_dir/../..")" -qiskit_venv="$(pwd -P)/qiskit_venv" +# First, prepare a wheel file for the dev version. We install several venvs with this, and while +# cargo will cache some rust artefacts, it still has to re-link each time, so the wheel build takes +# a little while. +wheel_dir="$(pwd -P)/wheels" +python -m pip wheel --no-deps --wheel-dir "$wheel_dir" "$repo_root" +all_wheels=("$wheel_dir"/*.whl) +qiskit_dev_wheel="${all_wheels[0]}" + +# Now set up a "base" development-version environment, which we'll use for most of the backwards +# compatibility checks. +qiskit_venv="$(pwd -P)/venvs/dev" qiskit_python="$qiskit_venv/bin/python" python -m venv "$qiskit_venv" + # `packaging` is needed for the `get_versions.py` script. -# 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_venv/bin/pip" install -c "$repo_root/constraints.txt" "$qiskit_dev_wheel" packaging +# Run all of the tests of cross-Qiskit-version compatibility. "$qiskit_python" "$our_dir/get_versions.py" | parallel --colsep=" " bash "$our_dir/process_version.sh" -p "$qiskit_python" -# Test dev compatibility +# Test dev compatibility with itself. dev_version="$("$qiskit_python" -c 'import qiskit; print(qiskit.__version__)')" +mkdir -p "dev-files/base" +pushd "dev-files/base" "$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 + + +# Test dev compatibility with all supported combinations of symengine between generator and loader. +# This will likely duplicate the base dev-compatibility test, but the tests are fairly fast, and +# it's better safe than sorry with the serialisation tests. + +symengine_versions=( + '>=0.11,<0.12' + '>=0.13,<0.14' +) +symengine_venv_prefix="$(pwd -P)/venvs/dev-symengine-" +symengine_files_prefix="$(pwd -P)/dev-files/symengine-" + +# Create the venvs and QPY files for each symengine version. +for i in "${!symengine_versions[@]}"; do + specifier="${symengine_versions[$i]}" + symengine_venv="$symengine_venv_prefix$i" + files_dir="$symengine_files_prefix$i" + python -m venv "$symengine_venv" + "$symengine_venv/bin/pip" install -c "$repo_root/constraints.txt" "$qiskit_dev_wheel" "symengine$specifier" + mkdir -p "$files_dir" + pushd "$files_dir" + "$symengine_venv/bin/python" -c 'import symengine; print(symengine.__version__)' > "SYMENGINE_VERSION" + "$symengine_venv/bin/python" "$our_dir/test_qpy.py" generate --version="$dev_version" + popd +done + +# For each symengine version, try loading the QPY files from every other symengine version. +for loader_num in "${!symengine_versions[@]}"; do + loader_venv="$symengine_venv_prefix$loader_num" + loader_version="$(< "$symengine_files_prefix$loader_num/SYMENGINE_VERSION")" + for generator_num in "${!symengine_versions[@]}"; do + generator_files="$symengine_files_prefix$generator_num" + generator_version="$(< "$generator_files/SYMENGINE_VERSION")" + echo "Using symengine==$loader_version to load files generated with symengine==$generator_version" + pushd "$generator_files" + "$loader_venv/bin/python" "$our_dir/test_qpy.py" load --version="$dev_version" + popd + done +done