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

Revert "Use libblastrampoline to forward to a user-defined BLAS at runtime" #39842

Closed
wants to merge 1 commit into from

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Feb 26, 2021

Reverts #39455

This seems to have broken one or more packages (as well as missing some checksums?)

@DilumAluthge
Copy link
Member

😢

Hopefully we can re-add the functionality in time for 1.7.

@KristofferC
Copy link
Member

What packages? Why are they broken? From what I see, the packages where accessing internals so the package should be fixed.

@ViralBShah
Copy link
Member

ViralBShah commented Feb 26, 2021

Don't the missing checksums get auto-computed? It is easier to add the missing checksums than revert this whole PR. It would be good to understand which packages have been affected. @KristofferC helped do a pretty complete audit - but it is possible something has been missed. My understanding is that it is only a handful of packages that are accessing internal APIs and they will be updated in the 1.7 cycle.

@ViralBShah ViralBShah marked this pull request as draft February 26, 2021 23:43
@simeonschaub
Copy link
Member

It looks like the failures were mainly due to NNlib, which ccalls libblas directly.

@ViralBShah
Copy link
Member

ViralBShah commented Feb 27, 2021

We'll get NNLib fixed. You have to get the changes into Julia master early on in a release cycle and then work through the ecosystem. In fact much of Yggdrasil needs to be updated to link against LBT in any case (but obviously most things won't break since they are just directly linking to openblas).

cc @DhairyaLGandhi on NNLib.jl.

@ViralBShah
Copy link
Member

ViralBShah commented Feb 27, 2021

I see what happened. After the pkgeval audit, we ended up removing the internal BLAS.libblas name and calling it BLAS.libblastrampoline. This is causing NNlib.jl to fail and a whole bunch of stuff that depends on it.

936e84f

@staticfloat

@ViralBShah
Copy link
Member

Fixes coming in #39845

Can always reopen this if necessary.

@ViralBShah ViralBShah closed this Feb 27, 2021
@vtjnash vtjnash deleted the revert-39455-vs/trampolineblas branch February 27, 2021 00:49
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.

5 participants