Skip to content

Updated for Julia 1.0, added (limited) auto detection of libraries and avx support #17

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

Merged
merged 30 commits into from
Nov 26, 2019

Conversation

Crown421
Copy link
Collaborator

@Crown421 Crown421 commented Sep 4, 2019

No description provided.

@aminya
Copy link
Member

aminya commented Nov 13, 2019

Is there any update on this, MKL was updated recently https://github.com/JuliaComputing/MKL.jl, but it doesn't use VML functions (like exp, log) by itself.

Copy link
Member

@aminya aminya left a comment

Choose a reason for hiding this comment

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

It is loaded and added without any issues now. Thanks!

@aminya
Copy link
Member

aminya commented Nov 13, 2019

How can I call the functions? I don't get any speed up. Is the Base API replaced, and so I can call the functions like just normal names (log, exp, etc)?

@Crown421
Copy link
Collaborator Author

VML extends the base functions to e.g. sin(x::Array{Float64}), as the base functions don't take arrays but instead rely on broadcasting.

julia> using VML
julia> a = rand(10000);
julia>@time  sin.(a);                                                                                                    
 0.159878 seconds (583.25 k allocations: 30.720 MiB, 2.78% gc time)
julia> @time sin(a);                                                                                                      
0.000465 seconds (6 allocations: 781.484 KiB) 

In short, if you call function with an array they go to vml, otherwise they use the base functions.

@aminya
Copy link
Member

aminya commented Nov 13, 2019

Oh yes, I was trying the wrong code! I thought it is possible to run something like sin(a) in Julia Base. It would be nice to add this example to the Readme!

@mcabbott
Copy link
Contributor

Great to see progress!

VML extends the base functions to e.g. sin(x::Array{Float64}), as the base functions don't take arrays

I didn't realise it did this, are we sure this the right way to go? Overloading Base functions on Base types. You must write Yeppp.sin and AppleAccelerate.exp etc. Perhaps the 1.0 update is a good moment to do likewise here?

@aminya
Copy link
Member

aminya commented Nov 13, 2019

Great to see progress!

VML extends the base functions to e.g. sin(x::Array{Float64}), as the base functions don't take arrays

I didn't realise it did this, are we sure this the right way to go? Overloading Base functions on Base types. You must write Yeppp.sin and AppleAccelerate.exp etc. Perhaps the 1.0 update is a good moment to do likewise here?

I think this is beyond this pull request, and it will just lag using this package. Changing the API requires a completely different work

@Crown421
Copy link
Collaborator Author

Great to see progress!

VML extends the base functions to e.g. sin(x::Array{Float64}), as the base functions don't take arrays

I didn't realise it did this, are we sure this the right way to go? Overloading Base functions on Base types. You must write Yeppp.sin and AppleAccelerate.exp etc. Perhaps the 1.0 update is a good moment to do likewise here?

It does, when I updated the test suite I ran into some issues verifying that a given call actually belongs to Base or VML, to make sure that I am not comparing a function to itself in terms of accurate output.
Ideally I would like the option that using VML extends Base and import VML doesn't, like one would expect, but unfortunately there seem to be some restrictions in the way Julia itself works here.
I think that is appropriate for its own issue.

@mcabbott
Copy link
Contributor

I can't seem to make this work, with MKL.jl. On a mac with Julia 1.3rc4 or 1.4, afetr installing MKL.jl, I can add & build VML (Crown421:master), but can't use it. Is this expected to work?

julia> using VML
[ Info: Precompiling VML [c8ce9da6-5d36-5c03-b118-5a70151be7bc]
ERROR: LoadError: LoadError: could not load library "libmkl_rt"
dlopen(libmkl_rt.dylib, 9): image not found
Stacktrace:
 [1] #dlopen#3(::Bool, ::typeof(Libdl.dlopen), ::String, ::UInt32) at /Applications/Julia-1.3.app/Contents/Resources/julia/lib/julia/sys.dylib:?
 [2] #dlopen#2 at /Applications/Julia-1.3.app/Contents/Resources/julia/share/julia/stdlib/v1.3/Libdl/src/Libdl.jl:109 [inlined]
 [3] dlopen at /Applications/Julia-1.3.app/Contents/Resources/julia/share/julia/stdlib/v1.3/Libdl/src/Libdl.jl:105 [inlined]
 [4] __init__() at /Users/me/.julia/dev/VML/src/setup.jl:3

julia> using Libdl

julia> dlopen(:libmkl_rt; throw_error=false) !== nothing
false

@Crown421
Copy link
Collaborator Author

Crown421 commented Nov 21, 2019

@mcabbott Try using it now, I think I made it work now reasonably well.
Readme comes next.

@aminya
Copy link
Member

aminya commented Nov 21, 2019

Thank you guys for the work.

I will test the functionality.

Adding MKL to before_script

Avoiding double build of MKL

Try again to avoid double build

Try again to avoid double build 2

Maybe with a Manifest

Test specific environment

Add MKL again

Basically final

final
@Crown421
Copy link
Collaborator Author

@KristofferC Would it be possible to merge this PR now?
While still not perfect, I think this package is now ready for use.

@aminya
Copy link
Member

aminya commented Nov 23, 2019

I am going to create a PR to this branch and update the benchmarks code.
https://github.com/aminya/VML.jl/tree/workingJulia1

@aminya
Copy link
Member

aminya commented Nov 23, 2019

The pull request is ready for merging! Crown421#2

Crown421 and others added 2 commits November 24, 2019 14:28
* updating used deps

* bench function update

* ratioci function formatting

* input changed to Dict and updated

* fns updated

* single bench call

* Docstring for bench function

* Using Dict for different types in benchmark

* Formatting

* Benchmarking progress print

* using Dicts for calling function names

* example for calling bench

* bench call updated

* type print in bench

* Full working bench example

* Plot function updated

* Code movement

* Saving benchmark data

* DPI increase

* Full test

* more docstrings

* changed variable size

* Disabled error bar calculation

* removing log10 and lgamma

* correct hline legend

* code for loading the data

* Adding benchmark result

* bug fix

* Update performance.png

* Fixed title and ylabel

* cd to current file

* Update image

* git ignore jld2
@aminya
Copy link
Member

aminya commented Nov 25, 2019

@Crown421 I am working on making AcuteBenchmark by using the approach we took for benchmarking the functions that get arrays. I will add some features such as trying different sizes, etc. So, for now, don't spend time on changing things in the benchmark code. Once that package is finished we can do a more extensive benchmark for VML.

@Crown421
Copy link
Collaborator Author

I am not entirely sure what you mean with "approach for benchmarking the arrays" ?

@aminya
Copy link
Member

aminya commented Nov 26, 2019

That package is going to generate random arrays (as inputs) and benchmark the functions based on that. Similar to what we did in this package, but written in a general and reusable manner and with more features. I will post the source code once it is finished.

@Crown421
Copy link
Collaborator Author

Feel free to check out the benchmarks branch on my repo, which contains my first attempt at a rewrite (bm.jl and bm_functions.jl) from last week.
It also has some stuff for a more thorough job (bm_extended.jl) from Sunday, which includes testing whether the speed-up is statistically significant, but currently suffers from unexplained high RAM usage.

@aminya
Copy link
Member

aminya commented Nov 26, 2019

Thank you. That is useful. I also have some code from Julia-Matlab-Benchmark, which overall will make a useful package (AcuteBenchmark).

@aminya
Copy link
Member

aminya commented Nov 26, 2019

@simonster @andreasnoack @ViralBShah Could you merge this PR?

@ViralBShah ViralBShah merged commit ed542eb into JuliaMath:master Nov 26, 2019
@ViralBShah
Copy link
Member

@Crown421 @aminya I have invited you to have write access to the repo. Hope that helps with the management of this repo and keeping it updated. Thank you for the contributions!

@aminya
Copy link
Member

aminya commented Nov 26, 2019

Thank you! You're welcome.

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