Skip to content

Move aarch64 testing to native runner#17024

Merged
pllim merged 1 commit intoastropy:mainfrom
astrofrog:native-arm64-testing
Sep 17, 2024
Merged

Move aarch64 testing to native runner#17024
pllim merged 1 commit intoastropy:mainfrom
astrofrog:native-arm64-testing

Conversation

@astrofrog
Copy link
Member

@astrofrog astrofrog commented Sep 17, 2024 •
edited by pllim
Loading

This should shorten the job time from 6h to under 10 minutes 😆

Just FYI the parallel build is not twice as fast as serial, but we have to pay for a 2 core machine anyway and it does reduce the runtime from 11 to 9 minutes, so worth it anyway.

Fix #16565

It might be worth investigating whether qemu for armv7 is faster on arm64 than on intel chips, but that can wait.

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@astrofrog astrofrog added this to the v6.1.4 milestone Sep 17, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Sep 17, 2024

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@pllim pllim mentioned this pull request Sep 17, 2024
Open
pllim
pllim reviewed Sep 17, 2024
View reviewed changes
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Yay, 9 mins! I opened #17025 for the armv7 question you posed above.

Should we revert #16417 ?

Do you want to also address #11731 while you are at it?

.github/workflows/ci_cron_weekly.yml
@@ -171,3 +170,24 @@ jobs:
ASTROPY_USE_SYSTEM_ALL=1 pip3 install -v --no-build-isolation -e .[test]
Copy link
Member

@pllim pllim Sep 17, 2024

Choose a reason for hiding this comment

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

Do we not want to port over all the extra commands from the "exotic" matrix?

Copy link
Member Author

@astrofrog astrofrog Sep 17, 2024

Choose a reason for hiding this comment

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

No, those were only needed because we were trying to also link against the system libraries I think to speed up compilation, but no reason we need to do this here. The remaining archs are still good tests of that external linking system.

@astrofrog
Copy link
Member Author

astrofrog commented Sep 17, 2024

No point reverting #16417 as I do think we want to skip hypothesis in the emulated architectures, and this present PR does unskip hypothesis tests for arm64.

#11731 is more complicated because re-usable workflows from OpenAstronomy can't be used in paid runners in a different org, @Cadair is going to be experimenting with solutions on the sunpy side but let's not let it hold this back.

@astrofrog astrofrog requested a review from pllim September 17, 2024 13:12
pllim
pllim approved these changes Sep 17, 2024
View reviewed changes
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Well, I like going from time out to 9 mins. So let's get this in. Thank you!!!

@pllim pllim merged commit fe293ff into astropy:main Sep 17, 2024
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Sep 17, 2024
@pllim @meeseeksmachine
Backport PR astropy#17024: Move aarch64 testing to native runner
9ca4589
@meeseeksmachine meeseeksmachine mentioned this pull request Sep 17, 2024
Merged
@pllim pllim mentioned this pull request Sep 17, 2024
Closed
4 tasks
pllim added a commit that referenced this pull request Sep 17, 2024
@pllim
Merge pull request #17028 from meeseeksmachine/auto-backport-of-pr-17…
64ca0a1 
…024-on-v6.1.x

Backport PR #17024 on branch v6.1.x (Move aarch64 testing to native runner)
@pllim
Copy link
Member

pllim commented Sep 17, 2024

We already blew $0.22 in 45 minutes. 😉

Screenshot 2024-09-17 105204

@Cadair
Copy link
Member

Cadair commented Sep 17, 2024

We will be broke before we know it. SHUT IT DOWN.

@neutrinoceros neutrinoceros mentioned this pull request Sep 23, 2024
Merged
1 task
@neutrinoceros neutrinoceros mentioned this pull request Oct 5, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pllim pllim pllim approved these changes

Assignees

No one assigned

Labels

Extra CI Run cron CI as part of PR no-changelog-entry-needed Performance testing

Projects

None yet

Milestone

v6.1.4

Development

Successfully merging this pull request may close these issues.

TST: aarch64 job is timing out again

3 participants

@astrofrog @pllim @Cadair