Skip to content

Commit

Permalink
Move QPY tests to GitHub Actions and increase inter-symengine tests (#…
Browse files Browse the repository at this point in the history
…13273) (#13380)

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

(cherry picked from commit af8be25)

Co-authored-by: Jake Lishman <[email protected]>
  • Loading branch information
mergify[bot] and jakelishman authored Oct 29, 2024
1 parent fd18b99 commit 91e83ef
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 49 deletions.
25 changes: 2 additions & 23 deletions .azure/lint_docs_qpy-linux.yml → .azure/lint_docs-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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'
39 changes: 39 additions & 0 deletions .github/workflows/qpy.yml
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}

Expand Down Expand Up @@ -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 }}

Expand Down
3 changes: 3 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions test/qpy_compat/process_version.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
80 changes: 59 additions & 21 deletions test/qpy_compat/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,42 +14,80 @@
set -e
set -o pipefail
set -x
shopt -s nullglob

# Set fixed hash seed to ensure set orders are identical between saving and
# loading.
export PYTHONHASHSEED=$(python -S -c "import random; print(random.randint(1, 4294967295))")
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

0 comments on commit 91e83ef

Please sign in to comment.