-
Notifications
You must be signed in to change notification settings - Fork 377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify Aer CI configuration #1448
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently the aer ci configuration is too onerous and prevents us from making forward progress on development because we're constantly having to adapt to upstream changes or system errors in CI. This has been blocking us from potentially getting the 0.10.3 bugfix release prepared. To improve the situation and make ci maintainable this commit makes a few key changes to the CI system. The first is to stop installing qiskit-terra from source. While the goal when this was done was to catch failures from terra sooner in practice this just means we're playing catch up constantly. Aer has a much slower patch rate compare to qiskit-terra and in practice all doing this means is we have to fix ci anytime someone pushes a new PR because things have bit rotted. These breakages aren't aer's fault (at least normally) and are caused by breaking api changes or bugs introduced during the development of terra, which only guarantees api stability on releases. So we should treat terra just like any other upstream dependency and install it from it's release. Hopefully when we have proper integration testing setup in terra the spurious failures will decrease as well. The second key change made here is to decrease the duplication between jobs. In aer historically we've tested every permutation of install method on platform and python version. This was in response to early versions of aer which were packaged incorrectly and quite poorly and had issues installing everywhere without issue. However, this has led to a proliferation of jobs that are testing the same thing in slightly different ways. Additionally, because of all these jobs and limitations in github actions around retying failed jobs things were split into 3 workflows to enable retrying subsets of jobs by platform. This however further duplicated the jobs so that we had fast failures on each workflow. This commit undoes this, it reduces the 3 test workflows back into a single shared one. In that workflow we only have 1 lint job, one set of sdist jobs, 1 json input test, and then 1 platform specific set of unittest runs. This should hopefully decrease the wait time for CI and also make things easier to manage moving forward.
jakelishman
reviewed
Feb 4, 2022
jakelishman
approved these changes
Feb 4, 2022
mtreinish
added
the
stable-backport-potential
The issue or PR might be minimal and/or import enough to backport to stable
label
Feb 4, 2022
@mtreinish can we keep a workflow to test the latest terra as optional? I know other integration tests should cover tests with the latest qiskit-*. But, maybe, it is good to know what tests will be failed with the latest-terra in this repository. |
hhorii
pushed a commit
that referenced
this pull request
Feb 9, 2022
* Simplify Aer CI configuration Currently the aer ci configuration is too onerous and prevents us from making forward progress on development because we're constantly having to adapt to upstream changes or system errors in CI. This has been blocking us from potentially getting the 0.10.3 bugfix release prepared. To improve the situation and make ci maintainable this commit makes a few key changes to the CI system. The first is to stop installing qiskit-terra from source. While the goal when this was done was to catch failures from terra sooner in practice this just means we're playing catch up constantly. Aer has a much slower patch rate compare to qiskit-terra and in practice all doing this means is we have to fix ci anytime someone pushes a new PR because things have bit rotted. These breakages aren't aer's fault (at least normally) and are caused by breaking api changes or bugs introduced during the development of terra, which only guarantees api stability on releases. So we should treat terra just like any other upstream dependency and install it from it's release. Hopefully when we have proper integration testing setup in terra the spurious failures will decrease as well. The second key change made here is to decrease the duplication between jobs. In aer historically we've tested every permutation of install method on platform and python version. This was in response to early versions of aer which were packaged incorrectly and quite poorly and had issues installing everywhere without issue. However, this has led to a proliferation of jobs that are testing the same thing in slightly different ways. Additionally, because of all these jobs and limitations in github actions around retying failed jobs things were split into 3 workflows to enable retrying subsets of jobs by platform. This however further duplicated the jobs so that we had fast failures on each workflow. This commit undoes this, it reduces the 3 test workflows back into a single shared one. In that workflow we only have 1 lint job, one set of sdist jobs, 1 json input test, and then 1 platform specific set of unittest runs. This should hopefully decrease the wait time for CI and also make things easier to manage moving forward. * Add missing requirements to lint job defintion * Add missing windows test install step * Remove unecessary job run split and remove unused env vars
hhorii
pushed a commit
to hhorii/qiskit-aer
that referenced
this pull request
Feb 9, 2022
* Simplify Aer CI configuration Currently the aer ci configuration is too onerous and prevents us from making forward progress on development because we're constantly having to adapt to upstream changes or system errors in CI. This has been blocking us from potentially getting the 0.10.3 bugfix release prepared. To improve the situation and make ci maintainable this commit makes a few key changes to the CI system. The first is to stop installing qiskit-terra from source. While the goal when this was done was to catch failures from terra sooner in practice this just means we're playing catch up constantly. Aer has a much slower patch rate compare to qiskit-terra and in practice all doing this means is we have to fix ci anytime someone pushes a new PR because things have bit rotted. These breakages aren't aer's fault (at least normally) and are caused by breaking api changes or bugs introduced during the development of terra, which only guarantees api stability on releases. So we should treat terra just like any other upstream dependency and install it from it's release. Hopefully when we have proper integration testing setup in terra the spurious failures will decrease as well. The second key change made here is to decrease the duplication between jobs. In aer historically we've tested every permutation of install method on platform and python version. This was in response to early versions of aer which were packaged incorrectly and quite poorly and had issues installing everywhere without issue. However, this has led to a proliferation of jobs that are testing the same thing in slightly different ways. Additionally, because of all these jobs and limitations in github actions around retying failed jobs things were split into 3 workflows to enable retrying subsets of jobs by platform. This however further duplicated the jobs so that we had fast failures on each workflow. This commit undoes this, it reduces the 3 test workflows back into a single shared one. In that workflow we only have 1 lint job, one set of sdist jobs, 1 json input test, and then 1 platform specific set of unittest runs. This should hopefully decrease the wait time for CI and also make things easier to manage moving forward. * Add missing requirements to lint job defintion * Add missing windows test install step * Remove unecessary job run split and remove unused env vars
hhorii
added a commit
to hhorii/qiskit-aer
that referenced
this pull request
Feb 9, 2022
This changes installation of qiskit-terra in tox. Recent commit of Qiskit#1448 changed installation of qiskit-terra to use pypi in most CI workflow, but not tox.
mtreinish
pushed a commit
that referenced
this pull request
Feb 20, 2022
* stop installing stop installing qiskit-terra from source This changes installation of qiskit-terra in tox. Recent commit of #1448 changed installation of qiskit-terra to use pypi in most CI workflow, but not tox. * reduce explicit installation of qiskit-terra in tox.ini Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
stable-backport-potential
The issue or PR might be minimal and/or import enough to backport to stable
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Currently the aer ci configuration is too onerous and prevents us from
making forward progress on development because we're constantly having
to adapt to upstream changes or system errors in CI. This has been
blocking us from potentially getting the 0.10.3 bugfix release prepared.
To improve the situation and make ci maintainable this commit makes a
few key changes to the CI system. The first is to stop installing
qiskit-terra from source. While the goal when this was done was to catch
failures from terra sooner in practice this just means we're playing
catch up constantly. Aer has a much slower patch rate compare to
qiskit-terra and in practice all doing this means is we have to fix ci
anytime someone pushes a new PR because things have bit rotted. These
breakages aren't aer's fault (at least normally) and are caused by
breaking api changes or bugs introduced during the development of terra,
which only guarantees api stability on releases. So we should treat
terra just like any other upstream dependency and install it from it's
release. Hopefully when we have proper integration testing setup in
terra the spurious failures will decrease as well. The second key change
made here is to decrease the duplication between jobs. In aer
historically we've tested every permutation of install method on
platform and python version. This was in response to early versions of
aer which were packaged incorrectly and quite poorly and had issues
installing everywhere without issue. However, this has led to a
proliferation of jobs that are testing the same thing in slightly
different ways. Additionally, because of all these jobs and limitations
in github actions around retying failed jobs things were split into 3
workflows to enable retrying subsets of jobs by platform. This however
further duplicated the jobs so that we had fast failures on each
workflow. This commit undoes this, it reduces the 3 test workflows back
into a single shared one. In that workflow we only have 1 lint job, one
set of sdist jobs, 1 json input test, and then 1 platform specific set
of unittest runs. This should hopefully decrease the wait time for CI
and also make things easier to manage moving forward.
Details and comments