Skip to content
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

Add support for Python 3.11 #9028

Merged
merged 14 commits into from
Nov 3, 2022
Merged

Conversation

mtreinish
Copy link
Member

Summary

Python 3.11.0 was released on 10-24-2022, this commit marks the start of support for Python 3.11 in qiskit. It adds the supported Python version in the package metadata and updates the CI configuration to run test jobs on Python 3.11 and build Python 3.11 wheels on release.

Details and comments

Python 3.11.0 was released on 10-24-2022, this commit marks the start of
support for Python 3.11 in qiskit. It adds the supported Python version in
the package metadata and updates the CI configuration to run test jobs
on Python 3.11 and build Python 3.11 wheels on release.
@mtreinish mtreinish added on hold Can not fix yet stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: New Feature Include in the "Added" section of the changelog labels Oct 28, 2022
@mtreinish mtreinish requested a review from a team as a code owner October 28, 2022 20:10
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

Per the Python 3.11.0 release notes inspect.Parameter now raises a
ValueError if the name argument is a Python identifier. This was causing a
test failure in one case where a parameter named `lambda` was used.
This commit adjusts the parameter name in the tests to be lam to avoid
this issue.
@@ -393,15 +393,15 @@ def test_schedule_with_non_alphanumeric_ordering(self):
"""Test adding and getting schedule with non obvious parameter ordering."""
theta = Parameter("theta")
phi = Parameter("phi")
lamb = Parameter("lambda")
lamb = Parameter("lam")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. python/cpython#92062 is what is requiring the change here. If I understand correctly, InstrumentScheduleMap is using inspect.signature and inspect.Parameter for convenience internally but the schedule parameters do not need to form a valid Python signature (I don't see the signature being returned out of InstructionScheduleMap). So we could change the internals if we wanted to allow lambda as a parameter. lambda seems like a common name to want to use, but I think with BackendV2 we want to move away from InstructionScheduleMap any way, so it's probably not worth changing for Python 3.11.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all Alonzo Church's fault

@coveralls
Copy link

coveralls commented Oct 28, 2022

Pull Request Test Coverage Report for Build 3387089144

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 84.543%

Totals Coverage Status
Change from base Build 3386389563: 0.007%
Covered Lines: 62456
Relevant Lines: 73875

💛 - Coveralls

Currently jax doesn't publish Python 3.11 wheels which is blocking test
runs with python 3.11. Since jax is an optional package only used for
the gradient package we can just skip it as isn't a full blocker for
using python 3.11. This commit sets an environment marker on the jax
dev requirements to only try to install it on Python < 3.11.
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Nov 2, 2022
Im Qiskit#9042 we removed the special case code to enable using toqm as an
optional routing method. This was needed prior to qiskit-terra 0.22.0 as
we didn't have the transpiler stage plugin interface. However, now that
we've added a dedicated interface for external transpiler passes to
integrate into transpile() the toqm passes are using that interface.
However, in Qiskit#9042 the tests for verifying the previously hard-coded toqm
routing method path worked as before were left intact. In general this
would be a good approach to ensure we're maintaining backwards
compatibilty with the new modular interface with earlier releases.
However, in this specific case this is causing an issue with how tests
are run. Since qiskit-toqm has a necessary dependency on qiskit-terra
this means when we install our development requirements before running
tests toqm is pulling in the latest stable release of qiskit-terra from
pypi. Then we later upgrade that with the source build before running
tests. This however this bi-directional test dependency introduces a
tension in a number of places. For the most recent example, when we're
trying to add support for new platforms (see Qiskit#9028 for an example)
where the stable version of qiskit-terra does not support the platform.
We're unable to run CI with qiskit-toqm being installed first since
installing stable qiskit-terra won't work/

This commit removes qiskit-toqm from the requirements-dev.txt list and
also removes the test cases using it to fix this conflict. In general
the testing for qiskit-toqm can now be self contained since it's
exercising a stable interface in terra that's tested independently. When
weighing the backwards compatibility coverage vs the CI and build
complexities having the bidirectional test dependency has just removing
the toqm tests and development requirement is the simplest path forward.
@mtreinish
Copy link
Member Author

mtreinish commented Nov 2, 2022

This is effectively blocked by #9057 which is causing terra 0.22.x to be installed in CI prior to building terra 0.23.0 from main. Terra 0.22.x itself is actually compatible with python 3.11 if built from source, but the tweedledum dependency, which has been removed from main, is not easily built from source and fails when we try to install terra 0.22.x for toqm. Once #9057 merges I think this might be good to go, but we'll have to wait for CI to see if there are any other blockers.

@jakelishman jakelishman added this to the 0.22.2 milestone Nov 2, 2022
mergify bot added a commit that referenced this pull request Nov 2, 2022
* Remove qiskit-toqm tests and requirements-dev.txt entry

Im #9042 we removed the special case code to enable using toqm as an
optional routing method. This was needed prior to qiskit-terra 0.22.0 as
we didn't have the transpiler stage plugin interface. However, now that
we've added a dedicated interface for external transpiler passes to
integrate into transpile() the toqm passes are using that interface.
However, in #9042 the tests for verifying the previously hard-coded toqm
routing method path worked as before were left intact. In general this
would be a good approach to ensure we're maintaining backwards
compatibilty with the new modular interface with earlier releases.
However, in this specific case this is causing an issue with how tests
are run. Since qiskit-toqm has a necessary dependency on qiskit-terra
this means when we install our development requirements before running
tests toqm is pulling in the latest stable release of qiskit-terra from
pypi. Then we later upgrade that with the source build before running
tests. This however this bi-directional test dependency introduces a
tension in a number of places. For the most recent example, when we're
trying to add support for new platforms (see #9028 for an example)
where the stable version of qiskit-terra does not support the platform.
We're unable to run CI with qiskit-toqm being installed first since
installing stable qiskit-terra won't work/

This commit removes qiskit-toqm from the requirements-dev.txt list and
also removes the test cases using it to fix this conflict. In general
the testing for qiskit-toqm can now be self contained since it's
exercising a stable interface in terra that's tested independently. When
weighing the backwards compatibility coverage vs the CI and build
complexities having the bidirectional test dependency has just removing
the toqm tests and development requirement is the simplest path forward.

* Remove unused imports

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mergify bot pushed a commit that referenced this pull request Nov 2, 2022
* Remove qiskit-toqm tests and requirements-dev.txt entry

Im #9042 we removed the special case code to enable using toqm as an
optional routing method. This was needed prior to qiskit-terra 0.22.0 as
we didn't have the transpiler stage plugin interface. However, now that
we've added a dedicated interface for external transpiler passes to
integrate into transpile() the toqm passes are using that interface.
However, in #9042 the tests for verifying the previously hard-coded toqm
routing method path worked as before were left intact. In general this
would be a good approach to ensure we're maintaining backwards
compatibilty with the new modular interface with earlier releases.
However, in this specific case this is causing an issue with how tests
are run. Since qiskit-toqm has a necessary dependency on qiskit-terra
this means when we install our development requirements before running
tests toqm is pulling in the latest stable release of qiskit-terra from
pypi. Then we later upgrade that with the source build before running
tests. This however this bi-directional test dependency introduces a
tension in a number of places. For the most recent example, when we're
trying to add support for new platforms (see #9028 for an example)
where the stable version of qiskit-terra does not support the platform.
We're unable to run CI with qiskit-toqm being installed first since
installing stable qiskit-terra won't work/

This commit removes qiskit-toqm from the requirements-dev.txt list and
also removes the test cases using it to fix this conflict. In general
the testing for qiskit-toqm can now be self contained since it's
exercising a stable interface in terra that's tested independently. When
weighing the backwards compatibility coverage vs the CI and build
complexities having the bidirectional test dependency has just removing
the toqm tests and development requirement is the simplest path forward.

* Remove unused imports

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 8999047)

# Conflicts:
#	requirements-dev.txt
jakelishman
jakelishman previously approved these changes Nov 2, 2022
@mtreinish mtreinish removed the on hold Can not fix yet label Nov 2, 2022
@mtreinish
Copy link
Member Author

All the wheels builds succeeded so I think this should be good to go now (assuming the linux 3.11 test job passes this time)

jakelishman
jakelishman previously approved these changes Nov 2, 2022
@mtreinish
Copy link
Member Author

mtreinish commented Nov 2, 2022

I just realized this is going to fail on the Linux 3.11 CI job because of the qpy tests. We can't install historical versions of Qiskit with 3.11 so the old version installs will fail (because of tweedledum, if that wasn't a concern we'd just be compiling Qiskit many times). I think this is actually a flaw in our ci matrix that we run qpy backwards compat tests on the leading edge version instead of the trailing edge. But it's also a general flaw in the methodology, at some future date when 3.9 goes EoL we'll have no supported python version for terra 0.18.0 so installing that will be an issue. (I guess we'll just have to raise the minimum testing version when that happens)

Regardless, this is something we'll have to fix tomorrow.

This commit moves the qpy backwards compatibility testing from the
leading edge python version, which in this PR branch is Python 3.11, to
the trailing edge Python version which is currently 3.7. Trying to add
support for a new Python version has demonstrated that we can't use the
leading edge version as historical versions of Qiskit used to generate
old QPY payloads are not going to be generally installable with newer
Python versions. So by using the trailing edge version instead we can
install all the older versions of Qiskit as there is Python
compatibility for those Qiskit versions. Eventually we will need to
raise the minimum Qiskit version we use in the QPY tests, when Python
3.9 goes EoL in October 2025 and Qiskit Terra 0.18.0 no longer has any
supported versions of Python it was released for. We probably could
get by another year until Python 3.10 goes EoL in 2026 it just means
we're building 0.18.x and 0.19.x from source for the testing, but when
Python 3.11 becomes our oldest supported version we'll likely have to
bump the minimum version.

This does go a bit counter to the intent of the test matrix to make the
first stage return fast and do a more through check in the second stage.
But, in this case the extra runtime is worth the longer term stability
in the tests.
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to merge with this change for now, to get the release out.

@mergify mergify bot merged commit 75d66dd into Qiskit:main Nov 3, 2022
mergify bot pushed a commit that referenced this pull request Nov 3, 2022
* Add support for Python 3.11

Python 3.11.0 was released on 10-24-2022, this commit marks the start of
support for Python 3.11 in qiskit. It adds the supported Python version in
the package metadata and updates the CI configuration to run test jobs
on Python 3.11 and build Python 3.11 wheels on release.

* Fix inspect.Parameter usage for API change in 3.11

Per the Python 3.11.0 release notes inspect.Parameter now raises a
ValueError if the name argument is a Python identifier. This was causing a
test failure in one case where a parameter named `lambda` was used.
This commit adjusts the parameter name in the tests to be lam to avoid
this issue.

* Set a version cap on the jax dev requirement

Currently jax doesn't publish Python 3.11 wheels which is blocking test
runs with python 3.11. Since jax is an optional package only used for
the gradient package we can just skip it as isn't a full blocker for
using python 3.11. This commit sets an environment marker on the jax
dev requirements to only try to install it on Python < 3.11.

* Set python version cap on cplex in CI

* DNM: Test wheel builds work

* Skip tests on i686/win32 wheel buids with python 3.11

* Revert "DNM: Test wheel builds work"

This reverts commit 725c21b.

* Run QPY backwards compat tests on trailing edge Python version

This commit moves the qpy backwards compatibility testing from the
leading edge python version, which in this PR branch is Python 3.11, to
the trailing edge Python version which is currently 3.7. Trying to add
support for a new Python version has demonstrated that we can't use the
leading edge version as historical versions of Qiskit used to generate
old QPY payloads are not going to be generally installable with newer
Python versions. So by using the trailing edge version instead we can
install all the older versions of Qiskit as there is Python
compatibility for those Qiskit versions. Eventually we will need to
raise the minimum Qiskit version we use in the QPY tests, when Python
3.9 goes EoL in October 2025 and Qiskit Terra 0.18.0 no longer has any
supported versions of Python it was released for. We probably could
get by another year until Python 3.10 goes EoL in 2026 it just means
we're building 0.18.x and 0.19.x from source for the testing, but when
Python 3.11 becomes our oldest supported version we'll likely have to
bump the minimum version.

This does go a bit counter to the intent of the test matrix to make the
first stage return fast and do a more through check in the second stage.
But, in this case the extra runtime is worth the longer term stability
in the tests.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 75d66dd)
mergify bot added a commit that referenced this pull request Nov 3, 2022
…) (#9061)

* Remove qiskit-toqm tests and requirements-dev.txt entry (#9057)

* Remove qiskit-toqm tests and requirements-dev.txt entry

Im #9042 we removed the special case code to enable using toqm as an
optional routing method. This was needed prior to qiskit-terra 0.22.0 as
we didn't have the transpiler stage plugin interface. However, now that
we've added a dedicated interface for external transpiler passes to
integrate into transpile() the toqm passes are using that interface.
However, in #9042 the tests for verifying the previously hard-coded toqm
routing method path worked as before were left intact. In general this
would be a good approach to ensure we're maintaining backwards
compatibilty with the new modular interface with earlier releases.
However, in this specific case this is causing an issue with how tests
are run. Since qiskit-toqm has a necessary dependency on qiskit-terra
this means when we install our development requirements before running
tests toqm is pulling in the latest stable release of qiskit-terra from
pypi. Then we later upgrade that with the source build before running
tests. This however this bi-directional test dependency introduces a
tension in a number of places. For the most recent example, when we're
trying to add support for new platforms (see #9028 for an example)
where the stable version of qiskit-terra does not support the platform.
We're unable to run CI with qiskit-toqm being installed first since
installing stable qiskit-terra won't work/

This commit removes qiskit-toqm from the requirements-dev.txt list and
also removes the test cases using it to fix this conflict. In general
the testing for qiskit-toqm can now be self contained since it's
exercising a stable interface in terra that's tested independently. When
weighing the backwards compatibility coverage vs the CI and build
complexities having the bidirectional test dependency has just removing
the toqm tests and development requirement is the simplest path forward.

* Remove unused imports

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 8999047)

# Conflicts:
#	requirements-dev.txt

* Remove qiskit-toqm from requirements-dev.txt

This fixes a merge conflict and removes the qiskit-toqm entry from the requirements-dev.txt file

* Fix lint failure

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mergify bot added a commit that referenced this pull request Nov 3, 2022
* Add support for Python 3.11 (#9028)

* Add support for Python 3.11

Python 3.11.0 was released on 10-24-2022, this commit marks the start of
support for Python 3.11 in qiskit. It adds the supported Python version in
the package metadata and updates the CI configuration to run test jobs
on Python 3.11 and build Python 3.11 wheels on release.

* Fix inspect.Parameter usage for API change in 3.11

Per the Python 3.11.0 release notes inspect.Parameter now raises a
ValueError if the name argument is a Python identifier. This was causing a
test failure in one case where a parameter named `lambda` was used.
This commit adjusts the parameter name in the tests to be lam to avoid
this issue.

* Set a version cap on the jax dev requirement

Currently jax doesn't publish Python 3.11 wheels which is blocking test
runs with python 3.11. Since jax is an optional package only used for
the gradient package we can just skip it as isn't a full blocker for
using python 3.11. This commit sets an environment marker on the jax
dev requirements to only try to install it on Python < 3.11.

* Set python version cap on cplex in CI

* DNM: Test wheel builds work

* Skip tests on i686/win32 wheel buids with python 3.11

* Revert "DNM: Test wheel builds work"

This reverts commit 725c21b.

* Run QPY backwards compat tests on trailing edge Python version

This commit moves the qpy backwards compatibility testing from the
leading edge python version, which in this PR branch is Python 3.11, to
the trailing edge Python version which is currently 3.7. Trying to add
support for a new Python version has demonstrated that we can't use the
leading edge version as historical versions of Qiskit used to generate
old QPY payloads are not going to be generally installable with newer
Python versions. So by using the trailing edge version instead we can
install all the older versions of Qiskit as there is Python
compatibility for those Qiskit versions. Eventually we will need to
raise the minimum Qiskit version we use in the QPY tests, when Python
3.9 goes EoL in October 2025 and Qiskit Terra 0.18.0 no longer has any
supported versions of Python it was released for. We probably could
get by another year until Python 3.10 goes EoL in 2026 it just means
we're building 0.18.x and 0.19.x from source for the testing, but when
Python 3.11 becomes our oldest supported version we'll likely have to
bump the minimum version.

This does go a bit counter to the intent of the test matrix to make the
first stage return fast and do a more through check in the second stage.
But, in this case the extra runtime is worth the longer term stability
in the tests.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 75d66dd)

* Only require tweedledum on Python < 3.11

The tweedledum package is already treated as optional on the 0.22.x
release series and is not required for arm64 macOS systems. Tweedledum
doesn't yet support Python 3.11 so this commit restricts it as a
requirement for Python 3.11 users.

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@mtreinish mtreinish deleted the add-py311-support branch November 3, 2022 23:28
@1ucian0 1ucian0 mentioned this pull request Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants