Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5.24k stars 2.36k forks source link

Run test suite during release builds #12202

Closed mtreinish closed 1 month ago

mtreinish commented 6 months ago

What should we add?

Currently our release builds are configured to run this small test script: https://github.com/Qiskit/qiskit/blob/main/examples/python/stochastic_swap.py which executes the StochasticSwap transpiler pass. This is an historical artifact from before the use of Rust by qiskit, the stochastic swap pass was our only compiled extension (written in Cython) and the script was exercising solely that to speed up the validation phase of release builds. However in the intervening years more and more of Qiskit is being written in rust (with the 1.1.0 release poised to be >10% written in Rust by lines of code). We should update our wheel build configuration to instead run the unittests to validate that the binaries we publish work on all our supported python versions more broadly than just a single transpiler pass.

This primarily involves updating the cibuildwheel configuration: https://github.com/Qiskit/qiskit/blob/9ede1b6ace329b399fbf226b839df8190f1b4115/pyproject.toml#L136-L163 to run our unittests instead of the standalone script. We already run the tests once for PGO, but it would be good to validate that the wheels we publish pass the tests as part of the cibuildwheel test phase.

There is an open question, whether we should execute the full test suite or just a subset of the tests. I think for our tier 1 platforms (https://docs.quantum.ibm.com/start/install#operating-system-support) we probably should have enough CI time to run the full test suite. But for our tier 2 platforms this might be tricky because we run those builds under QEMU emulation. So for tier 2 platforms we might want a "smoke test" subset of the test suite that executes quicker because everything is very slow under emulation.

1ucian0 commented 5 months ago

This issue is participating on UnitaryHack 2024, between May 29 and June 12, 2024.

Because the nature of the event, there is no need to request to be assigned: just go ahead and PR your fix. The first/best PR can get the bounty (or it could be shared if they complement each other).

Erik-Chagas commented 5 months ago

Hi, I'm working on this issue as part of the unitaryHACK event. I'm unsure whether unittests and smoke tests (described as quicker tests for Tier 2 platforms) already exist in the Qiskit project and so we should just update cibuildwheel so that it runs the necessary unittests. Thank you in advance for your attention.

mtreinish commented 5 months ago

The unit test suite does already exist, that's what we run on every CI test job already. The smoke tests I mentioned don't exist as a category yet. It would be about selecting a limited subset of the larger complete test suite to run. That way we have some test coverage coverage but also a reasonable runtime.

Erik-Chagas commented 5 months ago

Hi, I thought about performing the tests using the 'tox' command for Tier 1 and 'tox -- tests/smoke' for Tier 2 Linux aarch64, changing the pyproject.toml file like this:

[tool.cibuildwheel]
manylinux-x86_64-image = "manylinux2014"
manylinux-i686-image = "manylinux2014"
skip = "pp* cp36-* cp37-* *musllinux* *win32 *i686 cp38-macosx_arm64"
test-skip = "*win32 *linux_i686"
test-command = "tox" # 'tox' command applied for testing
# We need to use pre-built versions of Numpy and Scipy in the tests; they have a
# tend to crash if they're installed from source by `pip install`, and since
# Numpy 1.22 there are no i686 wheels, so we force pip to use older ones without
# restricting any dependencies that Numpy and Scipy might have.
before-test = "pip install --only-binary=numpy,scipy numpy scipy"
# Some jobs locally override the before-build and environment configuration if a
# specific job override is needed. For example tier 1 platforms locally override
# the before-build and environment configuration to enable PGO,
# see: .github/workflows/wheels.yml for the jobs where this is done
environment = 'RUSTUP_TOOLCHAIN="stable"'

# New command session for Linux aarch64
[tool.cibuildwheel.linux.aarch64]
test-command = "tox -- tests/smoke"
before-all = "yum install -y wget && {package}/tools/install_rust.sh"
environment = 'PATH="$PATH:$HOME/.cargo/bin" CARGO_NET_GIT_FETCH_WITH_CLI="true" RUSTUP_TOOLCHAIN="stable"'
repair-wheel-command = "auditwheel repair -w {dest_dir} {wheel} && pipx run abi3audit --strict --report {wheel}"

[tool.cibuildwheel.linux]
before-all = "yum install -y wget && {package}/tools/install_rust.sh"
environment = 'PATH="$PATH:$HOME/.cargo/bin" CARGO_NET_GIT_FETCH_WITH_CLI="true" RUSTUP_TOOLCHAIN="stable"'
repair-wheel-command = "auditwheel repair -w {dest_dir} {wheel} && pipx run abi3audit --strict --report {wheel}"

[tool.cibuildwheel.macos]
environment = "MACOSX_DEPLOYMENT_TARGET=10.12"
repair-wheel-command = "delocate-wheel --require-archs {delocate_archs} -w {dest_dir} -v {wheel} && pipx run abi3audit --strict --report {wheel}"

[tool.cibuildwheel.windows]
repair-wheel-command = "copy {wheel} {dest_dir}/ && pipx run abi3audit --strict --report {wheel}"
mtreinish commented 5 months ago

That looks like the basic idea, the only issue is you can't use tox because that will trigger a build from source, the cibuildwheel flow is that they've built and installed the package at the point tests are run so the test-command runs in that environment with qiskit already installed. It probably would be best to update before-test to also add -r requirements-dev.txt to the pip install command and then call stestr directly in the test-command.

1ucian0 commented 3 months ago

UnitaryHACK 2024 finished. So this issue goes back to standard status.

Procatv commented 3 months ago

Can I be assigned to this issue, I'd like to take a look at it!