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

Fix recent CI errors in Neko and python 3.11 #1699

Closed
wants to merge 8 commits into from

Conversation

hhorii
Copy link
Collaborator

@hhorii hhorii commented Jan 11, 2023

Summary

Tests for python 3.11.1 are failed until the next terra release. Neko is failed because python -e . fails. This PR provides workaround for them.

Details and comments

This PR pins minor version of python 3.11 to 3.11.0 to pass all the tests.
Install Aer in Neko with python . (without -e option) and remove qiskit_aer directory to correctly refer the installed package.

Resolve #1698 and #1695.

@hhorii hhorii requested a review from mtreinish January 11, 2023 13:16
@mtreinish
Copy link
Member

I'm not quite sure what the goal is here, the 3.11 jobs are still failing because of their use of 3.11. Duplicating the job definition doesn't remove the jobs or change anything that I can tell. Is the intent here to stop running 3.11 tests? You would have to remove 3.11 from the list to do that. The primary workaround you can do while waiting on terra 0.22.4 (which should be out on Monday or Tuesday) is to set the version to 3.11.0 the error was introduced by 3.11.1 so explicitly setting the patch version should avoid this.

The other option is to decouple the base aer test class from qiskit's base test class. We arguably shouldn't be inheriting a test class like this across a library boundary exactly for reasons like this. In general test code you want to be as flat and explicit as possible and defining a bunch of core behavior outside the local test suite will just cause trouble in the long run. (it's also been on my list for a while to stop exposing this as a public interface in terra, see: Qiskit/qiskit#6862 )

@hhorii hhorii force-pushed the do_not_block_tests_with_311_tests branch from 3cc32ba to 8c72a2a Compare January 13, 2023 02:25
@hhorii hhorii changed the title Separate tests for python 3.11 from for other versions Set a minor version of 3.11 as 3.11.0 to work around errors due to 3.11.1 Jan 13, 2023
@hhorii hhorii force-pushed the do_not_block_tests_with_311_tests branch 3 times, most recently from 0017365 to 762c3a8 Compare January 13, 2023 07:25
@hhorii
Copy link
Collaborator Author

hhorii commented Jan 13, 2023

@mtreinish Thanks for your comments. I pinned python version to 3.11.0.

@hhorii hhorii changed the title Set a minor version of 3.11 as 3.11.0 to work around errors due to 3.11.1 Fix recent CI errors in Neko and python 3.11 Jan 13, 2023
@hhorii hhorii force-pushed the do_not_block_tests_with_311_tests branch from 7eb9736 to 688fab5 Compare January 13, 2023 08:09
@@ -14,4 +14,7 @@ jobs:
- uses: Qiskit/qiskit-neko@main
with:
test_selection: backend
repo_install_command: pip install -e .
repo_install_command: |
pip install -r requirements-dev.txt
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be needed, neko's tests have their own dev requirements (which get installed by the action definition) so we should only need to install aer from source to be able to run the tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mtreinish as shown in https://github.com/hhorii/qiskit-aer/actions/runs/3912462608/jobs/6687134955, installation of qiskit-aer fails because neko does not include conan in its requirements-dev.txt, I guess.

Copy link
Collaborator Author

@hhorii hhorii Jan 16, 2023

Choose a reason for hiding this comment

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

@mtreinish I confirmed that conan command is installed but cmake does not recognize it as executable. In my local env, following logs are appeared in pip install -v . but not in this CI.

    changing mode of /tmp/pip-build-env-hya3x7mj/overlay/bin/conan to 755
    changing mode of /tmp/pip-build-env-hya3x7mj/overlay/bin/conan_build_info to 755
    changing mode of /tmp/pip-build-env-hya3x7mj/overlay/bin/conan_server to 755

Usually, pip builds these commands in a temporary directory. I tried to change TMPDIR in aer installation but errors remained.

I think that installing conan manually before qiskit-aer installation in neko is a workaround. Could you give your thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is pointing to a wider issue in building from source. It looks like pip installing conan from pyproject.toml in that log but then cmake is not able to find it for some reason.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The current pyproject.toml install conan correctly in my local docker environment. I guess that it does not work well only in github actions for conan.

@hhorii hhorii force-pushed the do_not_block_tests_with_311_tests branch from 25471d3 to b1bbafc Compare January 16, 2023 08:09
@hhorii
Copy link
Collaborator Author

hhorii commented Jan 18, 2023

Now new terra 0.22.4 was released and 3.11.1 is no longer an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI fails in tests of python 3.11
2 participants