Skip to content

Add arm64 CI run to travis#9668

Merged
bsipocz merged 1 commit intoastropy:masterfrom
mhvk:travis-try-arm64
Mar 22, 2020
Merged

Add arm64 CI run to travis#9668
bsipocz merged 1 commit intoastropy:masterfrom
mhvk:travis-try-arm64

Conversation

@mhvk
Copy link
Contributor

@mhvk mhvk commented Nov 24, 2019

Following #9663, this tries also adding adds an arm64 run that does not use pip and hence may avoid some of the probems in #9407.

@mhvk
Copy link
Contributor Author

mhvk commented Nov 24, 2019

OK, this was an easy way to add an ARM build! At just over 16 minutes, the run is substantially faster than the custom pip installs in #9407.
Happily(?), I see at least some of the same errors as found by @aarchiba in #9661; will see if I can fix them.

@mhvk mhvk force-pushed the travis-try-arm64 branch 4 times, most recently from 3967e34 to d69de5b Compare November 24, 2019 22:47
@mhvk
Copy link
Contributor Author

mhvk commented Nov 24, 2019

Yay, with the fixes to time, this passes on arm! Rebasing to get all travis runs included.

@mhvk mhvk changed the title [DO NOT MERGE] Try adding an arm64 CI run to travis Addi an arm64 CI run to travis Nov 24, 2019
@mhvk mhvk added Bug time Affects-dev PRs and issues that do not impact an existing Astropy release and removed Experimental labels Nov 24, 2019
@mhvk mhvk modified the milestones: v4.0.1, v4.0 Nov 24, 2019
@mhvk
Copy link
Contributor Author

mhvk commented Nov 24, 2019

Moved the milestone since at least the test fixes for time should be included in 4.0 to help address the failures on arm64 that @aarchiba and @olebole were seeing (NOTE: the stats failure still needs addressing; would need testing with scipy??)

@mhvk mhvk changed the title Addi an arm64 CI run to travis Add arm64 CI run to travis Nov 25, 2019
@mhvk mhvk requested a review from bsipocz November 25, 2019 00:04
@bsipocz
Copy link
Member

bsipocz commented Nov 25, 2019

@mhvk - Do we need the full test suite run for both new jobs? I wonder whether they could also be merged with one of the other jobs, e.g to test an older version of numpy in addition to arm/bigendian?

@bsipocz
Copy link
Member

bsipocz commented Nov 25, 2019

(also, the conda-forge arm packaging is well under the way, and hopefully they'll sort out the installer problems, so we can just plug that into the existing infrastructure rather than installing everything manually)

@bsipocz
Copy link
Member

bsipocz commented Nov 25, 2019

So, I would suggest to keep the bug fixes here and factor out the travis changes in a separate PR that is not v4.0 critical. In fact I'm a bit worried about future headache that may come from the addition to conda/pip now we're introducing debian packaged python packages into our testing.

@mhvk
Copy link
Contributor Author

mhvk commented Nov 25, 2019

@bsipocz - It wasn't quite clear which job to use, but I'll look again (I had thought that we would just do this as a cron job, but here used a regular one to be sure it actually passed).

Anyway, I agree with moving out the fixes - we can then discuss the travis setup more at our leasure.

@aarchiba
Copy link
Contributor

Looks like the fixes have been moved to #9672

@pllim
Copy link
Member

pllim commented Nov 25, 2019

Since the fixes are moved, this doesn't need 4.0 milestone anymore?

@mhvk mhvk modified the milestones: v4.0, v4.1 Nov 25, 2019
@mhvk
Copy link
Contributor Author

mhvk commented Nov 25, 2019

Yes, fixes moved, and remilestoned.

@bsipocz bsipocz modified the milestones: v4.1, v4.0.1 Nov 25, 2019
@bsipocz bsipocz removed Affects-dev PRs and issues that do not impact an existing Astropy release Bug time labels Nov 25, 2019
@bsipocz
Copy link
Member

bsipocz commented Feb 20, 2020

@mhvk - could you try to rebase? I think it would be nice to see how long would it take now to run these new jobs, or maybe even experiment with the new conda-forge arm packages (though I expect pip might be still quicker).

@mhvk
Copy link
Contributor Author

mhvk commented Feb 21, 2020

Yes, I'll try rebasing. In the meantime, moving the milestone to 4.1, since there is no real hurry.

@mhvk mhvk modified the milestones: v4.0.1, v4.1 Feb 21, 2020
@mhvk mhvk force-pushed the travis-try-arm64 branch from d69de5b to 7c80338 Compare March 21, 2020 22:34
Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

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

Let's get this in, too.

@bsipocz
Copy link
Member

bsipocz commented Mar 22, 2020

I'll cherry-pick this to #10059. I feel putting the logic in based on the architecture is cleaner than here with the tox variables, but it can be done in a follow-up for master, too.

@bsipocz bsipocz merged commit bda418d into astropy:master Mar 22, 2020
@bsipocz
Copy link
Member

bsipocz commented Mar 22, 2020

Thanks @mhvk!

@bsipocz
Copy link
Member

bsipocz commented Mar 22, 2020

oh, and that now there are conda-forge arm builds, we can think about whether to use those or stay with apt installs.

My personal preferences would be to stick with pip, if not possible with conda, and only if we really must do apt installs of python packages.

@bsipocz
Copy link
Member

bsipocz commented Mar 22, 2020

@mhvk - it seems I wasn't careful enough. While the the s390 build was of normal speed (~8mins), the installs for this ARM one takes forever. Do you have suggestions how to speed it up?

@mhvk
Copy link
Contributor Author

mhvk commented Mar 22, 2020

@bsipocz - I think we should put this in cron too, so that the long duration doesn't matter so much (I had in fact planned to do that, but didn't get around to it yesterday).
Also agree that we should do it either based on architecture or based on a flag rather than TOXENV being empty. I'll try to make a clean-up PR later today.
Using one of them for pure pip might make sense, though really tox effectively does that already. apt is, I think, most useful for arm64 as it installs most directly.

@mhvk mhvk deleted the travis-try-arm64 branch March 22, 2020 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants