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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Downgrade Highway to latest released version (1.0.7) #25446

Merged
merged 1 commit into from
Dec 22, 2023

Conversation

Mousius
Copy link
Member

@Mousius Mousius commented Dec 21, 2023

We had to pull in some git commits after the initial Highway merge, but
it makes way more sense to use their well tested versions 馃樃

Hopefully fixes #25445

@rgommers
Copy link
Member

Thanks @Mousius. Let's let CI complete here, and then a rebase should make the aarch64 wheel builds on Cirrus run too. I'd be curious to see whether those pass with this update-to-latest, even if we decide to go back to the 1.0.7 release instead in the end.

@Mousius
Copy link
Member Author

Mousius commented Dec 21, 2023

Thanks @Mousius. Let's let CI complete here, and then a rebase should make the aarch64 wheel builds on Cirrus run too. I'd be curious to see whether those pass with this update-to-latest, even if we decide to go back to the 1.0.7 release instead in the end.

I don't think we can go back to 1.0.7, it's quite old and we've bumped a few times for fixes already. Potentially worth getting this in and then latching onto the next version release?

@rgommers
Copy link
Member

Everything seems happy here, so I've disabled non-Cirrus jobs in the rerun.

I don't think we can go back to 1.0.7, it's quite old and we've bumped a few times for fixes already. Potentially worth getting this in and then latching onto the next version release?

Sounds reasonable to me.

@Mousius
Copy link
Member Author

Mousius commented Dec 21, 2023

Looks like less errors, but still some errors. I'll try and take a look at this tomorrow.

@rgommers
Copy link
Member

Thanks!

Building locally on a manylinux aarch64 container will probably be more helpful that in CI. For macOS that's possible too with the Tart VM, see #25012 (comment).

@Mousius Mousius changed the title Bump Highway to latest version Downgrade Highway to latest released version Dec 22, 2023
@Mousius Mousius changed the title Downgrade Highway to latest released version Downgrade Highway to latest released version (1.0.7) Dec 22, 2023
We had to pull in some git commits after the initial Highway merge, but
it makes way more sense to use their well tested versions 馃樃

Hopefully fixes numpy#25445
@Mousius
Copy link
Member Author

Mousius commented Dec 22, 2023

@rgommers I checked out 1.0.7 on a c7g, and it actually seems ok. I think what happened is we started from a hash when VQSortStatic was introduced and didn't get the Highway release locked in, but it seems going back is alright.

I got mega test failures in ciwheelbuild on the instance but ran tests outside it, and they passed, so I think they're ok. Unsure what the difference is 馃

Apologies for the CI resource usage 馃檧 I had to re-rebase; hopefully, those jobs get shut down.

@rgommers rgommers added this to the 2.0.0 release milestone Dec 22, 2023
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Oh that's great, thanks for checking @Mousius! This looks much better, in it goes.

I'll follow up on the last couple of unrelated test failures in the aarch64 wheel builds jobs asap, so we can get back to fully green status.

@rgommers
Copy link
Member

Apologies for the CI resource usage 馃檧 I had to re-rebase; hopefully, those jobs get shut down.

No worries about this one at all. We try to be careful, but it's secondary to getting things to work:) We burned $15 in ~48 hours, which is fine - it's more the admin hassle of it than anything else.

@rgommers rgommers merged commit f3d12e7 into numpy:main Dec 22, 2023
88 of 93 checks passed
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.

BUG: aarch64 linux wheel builds failing on highway_qsort SVE error
2 participants