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

Increase timeout for Mac CI #1309

Merged
merged 6 commits into from
Nov 1, 2023

Conversation

coruscating
Copy link
Collaborator

@coruscating coruscating commented Nov 1, 2023

Summary

Mac CI tests can take significantly longer than Windows and Linux. This PR doubles the timeout for Mac tests so they don't fail as frequently and adds back the OMP_NUM_THREADS: 1 option, which seems to help reduce individual test times.

Copy link
Collaborator

@wshanks wshanks left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me. I also had to fix the TEST_TIMEOUT int issue in #1285. Did you want to increase the timeout for the terra-main run as well?

I wish we had more time to look into this. It would be nice to pull logs from a bunch of runs and do some data analysis on the variability of the test times. I think we might benefit from setting some variable to disable lower level parallelization which might be conflicting with stestr's parallelization, but I am not sure.

run: tox -e terra-main
if: runner.os == 'macOS'
env:
TEST_TIMEOUT: 120
Copy link
Collaborator

Choose a reason for hiding this comment

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

Have the cron tests been more reliable than the main tests? I don't see the OMP_NUM_THREADS setting in the main file and that's the kind of parallelization disabling that might help with interfering parallel 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.

Ah, just left a comment about the same thing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! If we had time, it might be good to look at this kind of thing more closely. Even though Windows and Linux are not timing out, they might still benefit from disabling parallelization of math operations like in numpy since stestr is already trying to use one cpu per test. We would need to look up the right environment variables and compare the run times.

@coruscating
Copy link
Collaborator Author

@wshanks good point, I added it to the terra-main run too. That workflow still has the OMP_NUM_THREADS: 1 option, and I haven't been getting Mac timeouts on terra-main runs, so maybe it's worth putting back into the main CI.

@coruscating coruscating added this pull request to the merge queue Nov 1, 2023
@coruscating coruscating removed this pull request from the merge queue due to a manual request Nov 1, 2023
@coruscating coruscating added this pull request to the merge queue Nov 1, 2023
Merged via the queue into qiskit-community:main with commit 12a71b9 Nov 1, 2023
10 checks passed
@coruscating coruscating deleted the mac-test-time branch November 1, 2023 21:16
nkanazawa1989 pushed a commit to nkanazawa1989/qiskit-experiments that referenced this pull request Jan 17, 2024
### Summary

Mac CI tests can take significantly longer than Windows and Linux. This
PR doubles the timeout for Mac tests so they don't fail as frequently
and adds back the `OMP_NUM_THREADS: 1` option, which seems to help
reduce individual test times.
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.

2 participants