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

upgrade OpenBLAS to 0.3.0 #27221

Merged
merged 1 commit into from
May 30, 2018
Merged

upgrade OpenBLAS to 0.3.0 #27221

merged 1 commit into from
May 30, 2018

Conversation

vchuravy
Copy link
Member

0.3.0 was released just recently and it has some improvement w.r.t to performance on Mac OSX.

@vchuravy vchuravy force-pushed the vc/openblas_upgrade branch from da7ae93 to 595f094 Compare May 23, 2018 16:13
@ararslan ararslan added the external dependencies Involves LLVM, OpenBLAS, or other linked libraries label May 23, 2018
@ViralBShah
Copy link
Member

Will the ci systems build openblas?

@ViralBShah
Copy link
Member

Note this also updates to lapack 3.8

@ViralBShah
Copy link
Member

FreeBSD died on Distributed. Appveyor looks like it might have timed out.

@ViralBShah
Copy link
Member

Restarted Appveyor build, but not sure if it will help.

@vchuravy
Copy link
Member Author

This might also benefit from #27222

@ViralBShah
Copy link
Member

Should we merge #27222 and rebase this on top of that?

@vchuravy vchuravy force-pushed the vc/openblas_upgrade branch from 595f094 to 0f7eb6e Compare May 23, 2018 21:59
@tkelman
Copy link
Contributor

tkelman commented May 24, 2018

Need to update the checksums.

@vchuravy
Copy link
Member Author

All of Travis stalled out. I was thinking that it might have been an issue with their infrastructure, but there is no report.

IIUC AV doesn't build OpenBLAS since it uses the prebuilt binaries. I kicked of some buildbot runs, but that this point our queue is 18h behind.

@ViralBShah
Copy link
Member

Pushed the checksums.

@vchuravy
Copy link
Member Author

5fcc6d3 already had the checksums

@andreasnoack
Copy link
Member

Would it make sense to start using https://github.com/staticfloat/OpenBLASBuilder/ instead of building these here?

@vchuravy
Copy link
Member Author

For Appveyor yes, since currently IIUC we are using the dll from the nightly.
We shouldn't move everything to BinaryBuilder since we need to test that the buildystem here is healthy.

(I won't be the person to change us to OpenBLASBuilder, though I have my hands full enough as it is)

@ViralBShah
Copy link
Member

@vchuravy I think I had an older branch, which is why I didn't see your checksum commit. Should have pulled.

@ViralBShah
Copy link
Member

How do we know whether the windows buildbot successfully built openblas 0.3 last night and that is being picked up here? The appveyor build logs seem to not have any indicating information about openblas.

@vchuravy
Copy link
Member Author

The buildbot builds haven even started yet, and they won't be used for the nightly.

I would propose waiting for them to go through and then merging this. At least I am not seeing any testfailures that are LinAlg related.

@vchuravy vchuravy requested a review from andreasnoack May 24, 2018 18:23
@vchuravy vchuravy force-pushed the vc/openblas_upgrade branch from 5d9f60a to 8ddb567 Compare May 24, 2018 18:38
@vchuravy vchuravy changed the title [WIP/RFC] upgrade OpenBLAS to 0.3.0 upgrade OpenBLAS to 0.3.0 May 24, 2018
Copy link
Member

@Sacha0 Sacha0 left a comment

Choose a reason for hiding this comment

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

🎉

@ViralBShah
Copy link
Member

ViralBShah commented May 25, 2018

This should fix #16653, JuliaLang/LinearAlgebra.jl#501, #25266 when merged.

@ViralBShah
Copy link
Member

I am marking as backport for 0.6.3 if @ararslan will allow it, as it will fix outstanding mac performance issues for 0.6 reported in the issues above.

@ararslan
Copy link
Member

I'd like to freeze the 0.6.3 backports at this point, otherwise it risks continuously amassing new commits while I try to resolve the one remaining issue.

@ararslan
Copy link
Member

Just because I'm curious: @nanosoldier runbenchmarks(ALL, vs=":master")

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @ararslan

@KristofferC
Copy link
Member

ERROR: LoadError: KeyError: key BenchmarkTools.TagFilter{getfield(Main, Symbol("##3#4"))}(getfield(Main, Symbol("##3#4"))()) not found
Stacktrace:
 [1] getindex at ./dict.jl:485 [inlined]
 [2] getindex(::BenchmarkTools.BenchmarkGroup, ::BenchmarkTools.TagFilter{getfield(Main, Symbol("##3#4"))}) at /home/nanosoldier/.julia/packages/BenchmarkTools/zEzk/src/groups.jl:31
 [3] top-level scope

@ViralBShah
Copy link
Member

Will nanosoldier build openblas 0.3 on this branch?

@vchuravy
Copy link
Member Author

vchuravy commented May 25, 2018

Will nanosoldier build openblas 0.3 on this branch?

Yes nanosoldier doesn't use any caching besides ccache.

@vchuravy vchuravy force-pushed the vc/openblas_upgrade branch from 8ddb567 to 649ac0c Compare May 29, 2018 17:41
@vchuravy vchuravy added this to the 0.7 milestone May 29, 2018
@vchuravy
Copy link
Member Author

@nanosoldier runbenchmarks(ALL, vs=":master")

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]

Logs and partial data can be found here
cc @ararslan

@ararslan
Copy link
Member

Nanosoldier is broken right now and I haven't yet figured out why.

@ViralBShah
Copy link
Member

ViralBShah commented May 29, 2018

Do we really need this on the 0.7 milestone? We can do this any time, given that it is completely non-breaking.

I will also note that in reality, it should be easy to merge this shortly.

@vchuravy vchuravy merged commit 9dc4a94 into master May 30, 2018
@vchuravy vchuravy deleted the vc/openblas_upgrade branch May 30, 2018 00:21
@StefanKarpinski StefanKarpinski modified the milestone: 0.7 May 30, 2018
@ViralBShah
Copy link
Member

We'll probably have openblas 0.3.1 by the time we release Julia 0.7. I have not yet seen anything major, and this looks like a good openblas release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external dependencies Involves LLVM, OpenBLAS, or other linked libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants