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

Use catalina for some osx jobs #3591

Merged
merged 19 commits into from
Feb 26, 2020
Merged

Conversation

mtreinish
Copy link
Member

@mtreinish mtreinish commented Dec 10, 2019

Summary

Right now we're only testing in CI that things work on macOS high sierra
but mojave and catalina have been out for a while and different osx versions
differ enough that it warrants testing the newest version works. This
commit switches the osx jobs in the to use catalina. Additionally, azure pipelines
is removing support for the high sierra environment which means we'll have
to migrate to a newer version anyway.

Details and comments

Right now we're only testing in CI that things work on macOS high sierra
but catalina has been out for a while and different osx versions differ
enough that it warrants testing that the new version works too. This
commit adds a new catalina job that runs only python 3.7 to verify that
things work on newer osx.
@mtreinish mtreinish changed the title Add catalina job Add mojave job Dec 10, 2019
@mtreinish mtreinish added the on hold Can not fix yet label Dec 10, 2019
azure-pipelines.yml Outdated Show resolved Hide resolved
@mtreinish
Copy link
Member Author

I'm actually thinking it might make more sense to just bump the existing osx jobs to mojave since this worked and is not a brand new release. I think the issues I was looking at before when we first migrated to azure must resolved now.

With the addition of python 3.8 jobs we're running at our job
concurrency limit. To avoid making the run time of ci too long by having
to wait on every commit this commit switches the second stage osx jobs
to use mojave instead of adding duplicated jobs on multiple osx
versions. We'll still use high sierra for the first stage's 3.7 testing
to maintain coverage there.
@mtreinish mtreinish removed the on hold Can not fix yet label Jan 8, 2020
@mtreinish mtreinish changed the title Add mojave job Use mojave for some osx jobs Jan 8, 2020
@mtreinish
Copy link
Member Author

It looks like this is failing because of: #2793 (which is why we ended up using high sierra in CI originally) There's not much we can do since the only consistent factor involved is osx 10.14 which we don't have any ability to influence and/or fix.

@nonhermitian
Copy link
Contributor

Conda install the needed numpy and scipy will work. They use mkl

@mtreinish
Copy link
Member Author

mtreinish commented Jan 25, 2020

The priority for this just went up, Azure Pipelines is going to stop offering the high sierra images starting March 23, 2020[1]. Which means we'll have to come up with a solution for running tests reliably on 10.14. I guess I'll have to pivot things to use conda instead of straight cpython and pip.

[1] https://devblogs.microsoft.com/devops/removing-older-images-in-azure-pipelines-hosted-pools/

Catalina images have been available on azure pipelines for a few days
now. This commit pivots to using those instead of 10.14 mojave which
have issues with segfaults because of system libraries.
@mtreinish
Copy link
Member Author

It looks like 10.15 is segfaulting too: https://dev.azure.com/qiskit-ci/qiskit-terra/_build/results?buildId=8099&view=logs&j=377f2c8d-8b6f-583f-e152-2ff025e2543a&t=28b3ff0d-9e76-5e94-89e9-eed7321a383e&l=4178 I guess I'll still have to switch it to conda with catalina

@mtreinish mtreinish changed the title Use mojave for some osx jobs Use catalina for some osx jobs Feb 7, 2020
@mtreinish mtreinish force-pushed the add-catalina-job branch 3 times, most recently from 0c98245 to 1f3ba87 Compare February 15, 2020 14:48
The pip installable numpy is causing segfaults on catalina and mojave
environments. To workaround this issue this commit switches to using a
conda install for the macos jobs only.
@mtreinish mtreinish assigned kdk and unassigned mtreinish Feb 24, 2020
@@ -234,8 +234,17 @@ stages:
inputs:
versionSpec: '$(python.version)'
displayName: 'Use Python $(python.version)'
- bash: echo "##vso[task.prependpath]$CONDA/bin"
displayName: Add conda to PATH
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it just echos the conda path. Maybe it doesn't need to be added to PATH?

Copy link
Member Author

Choose a reason for hiding this comment

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

kdk
kdk previously approved these changes Feb 24, 2020
@mtreinish mtreinish added the on hold Can not fix yet label Feb 24, 2020
@mtreinish
Copy link
Member Author

mtreinish commented Feb 24, 2020

It looks like this segfaulted again even with conda: https://dev.azure.com/qiskit-ci/qiskit-terra/_build/results?buildId=8866&view=logs&j=ce9a6331-8618-5e5b-ad8c-d3ab6621690a&t=b587a450-7392-5f12-37e1-082303ec0efb&l=4056 marking this as on hold until we figure this out

10.15 fails in the same way with or without conda, we've tried 10.14
without conda but not with it. This commit switches back to 10.14 now
that we've switched to using conda to see if the tests pass.
@nonhermitian nonhermitian mentioned this pull request Feb 25, 2020
mtreinish pushed a commit that referenced this pull request Feb 26, 2020
There is an OpenBlas threading issue affecting scipy.linalg.eigh that is holding up #3591. This removes the call to eigh and replaces it with eig that uses a different underlying blas call and just takes the real part of the resulting eigenvalues.

This also made explicit two of the tests that were causing issues as they were off by a minus sign in the test suite but were fine when run explicitly.

* fix threading crash

* fix tests to be explicit
Now that we have a fix for the random segfaulting in the quantum info
tests there shouldn't be a reason we need to keep running in conda. At
the same time this switches back to 10.15 to test the newest version of
macOS. This only reverted back to 10.14 briefly to see if the segfault
was isolated to 10.15, which it was not.
@mtreinish mtreinish removed the on hold Can not fix yet label Feb 26, 2020
@mtreinish
Copy link
Member Author

Thanks to @nonhermitian this is now working on all versions of python with or without conda. This is ready to go whenever now

@mtreinish mtreinish merged commit bca0193 into Qiskit:master Feb 26, 2020
@mtreinish mtreinish deleted the add-catalina-job branch February 26, 2020 22:08
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
There is an OpenBlas threading issue affecting scipy.linalg.eigh that is holding up Qiskit#3591. This removes the call to eigh and replaces it with eig that uses a different underlying blas call and just takes the real part of the resulting eigenvalues.

This also made explicit two of the tests that were causing issues as they were off by a minus sign in the test suite but were fine when run explicitly.

* fix threading crash

* fix tests to be explicit
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
Right now we're only testing in CI that things work on macOS high sierra
but mojave and catalina have been out for a while and different osx versions
differ enough that it warrants testing the newest version works. This
commit switches the osx jobs in the to use catalina. Additionally, azure pipelines
is removing support for the high sierra environment which means we'll have
to migrate to a newer version anyway.

* Add catalina job

Right now we're only testing in CI that things work on macOS high sierra
but catalina has been out for a while and different osx versions differ
enough that it warrants testing that the new version works too. This
commit adds a new catalina job that runs only python 3.7 to verify that
things work on newer osx.

* Update azure-pipelines.yml

Co-Authored-By: Kevin Krsulich <[email protected]>

* Use Mojave for all second stage osx jobs

With the addition of python 3.8 jobs we're running at our job
concurrency limit. To avoid making the run time of ci too long by having
to wait on every commit this commit switches the second stage osx jobs
to use mojave instead of adding duplicated jobs on multiple osx
versions. We'll still use high sierra for the first stage's 3.7 testing
to maintain coverage there.

* Pivot to macos 10.15 catalina

Catalina images have been available on azure pipelines for a few days
now. This commit pivots to using those instead of 10.14 mojave which
have issues with segfaults because of system libraries.

* Use conda for macOS jobs

The pip installable numpy is causing segfaults on catalina and mojave
environments. To workaround this issue this commit switches to using a
conda install for the macos jobs only.

* Install scipy via conda too

* Switch back to 10.14 again

10.15 fails in the same way with or without conda, we've tried 10.14
without conda but not with it. This commit switches back to 10.14 now
that we've switched to using conda to see if the tests pass.

* Switch back to 10.15 without conda

Now that we have a fix for the random segfaulting in the quantum info
tests there shouldn't be a reason we need to keep running in conda. At
the same time this switches back to 10.15 to test the newest version of
macOS. This only reverted back to 10.14 briefly to see if the segfault
was isolated to 10.15, which it was not.

Co-authored-by: Kevin Krsulich <[email protected]>
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.

3 participants