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

Add fix for llvm/llvm-project#61402 #209

Merged
merged 4 commits into from
Jun 2, 2023

Conversation

gmarkall
Copy link

@gmarkall gmarkall commented Apr 25, 2023

This addresses an issue that is triggered by Numba 0.57 - see numba/numba#8738

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

This addresses an issue that is triggered by Numba 0.57 - see
numba/numba#8738
@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member

Doesn't numba + LLVM 14 also need the SVML patches? I have an open PR for that that didn't get any comments: #192

Let's see if this compiles a bit faster after a rerender:
@conda-forge-admin, please rerender

@gmarkall
Copy link
Author

gmarkall commented Jun 1, 2023

Can we please review / merge this? It will be breaking many AArch64 users of Numba. For example, UMAP is unusable on AArch64: numba/numba#8994 / lmcinnes/umap#1004 (comment).

This patch has already been accepted upstream by LLVM but will not be in a release until 17.x:

@h-vetinari
Copy link
Member

Does this change make sense without SVML? I thought that was a requirement for using llvm 14 with numba, as things would break without SVML as well. #192 is waiting for action from the numba/llvmlite team.

@gmarkall
Copy link
Author

gmarkall commented Jun 1, 2023

This is unrelated to SVML. I don't know much about SVML but it's not required for Aarch64. So #192 should be considered completely independently of this.

@jakirkham
Copy link
Member

Think Graham's point here is simply this particular change has already gone upstream (and was merged). So may be more easily integrated than the other patch (orthogonal to this one), which hasn't quite made it upstream yet.

@h-vetinari
Copy link
Member

Yeah, I got that. I just wanted to confirm what's necessary for numba or not. I haven't had time to double-check with the upstream PR (can do sometime in the next ~12h), but if you want to go ahead and merge, I don't mind!

@jakirkham
Copy link
Member

Happy to wait 12hrs 🙂

Copy link
Member

@h-vetinari h-vetinari left a comment

Choose a reason for hiding this comment

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

Was on a phone yesterday which made this hard to check again. Thanks for the fix!

recipe/meta.yaml Outdated Show resolved Hide resolved
@h-vetinari h-vetinari merged commit 15f1d09 into conda-forge:14.x Jun 2, 2023
1 of 8 checks passed
@jakirkham
Copy link
Member

No worries. Been there done that 😅

Thanks Axel! 🙏

@gmarkall
Copy link
Author

gmarkall commented Jun 2, 2023

Many thanks for bumping the build number and the merge!

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.

None yet

3 participants