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

Test that we can use Apple Accelerate by default on Mac #124

Closed

Conversation

amontoison
Copy link
Member

@amontoison amontoison commented Oct 9, 2022

It's documented nowhere but I found this in the tests of LBT:
https://github.com/JuliaLinearAlgebra/libblastrampoline/blob/main/test/runtests.jl#L150-L165

We have CI with a M1 so we can test it.

@dpo
Copy link
Member

dpo commented Oct 9, 2022

The last time I checked, LAPACK was incomplete in Accelerate.

@dpo
Copy link
Member

dpo commented Oct 9, 2022

@JSOBot runtests

@codecov
Copy link

codecov bot commented Oct 9, 2022

Codecov Report

Base: 78.27% // Head: 81.59% // Increases project coverage by +3.32% 🎉

Coverage data is based on head (c843f00) compared to base (658bc69).
Patch coverage: 33.33% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #124      +/-   ##
==========================================
+ Coverage   78.27%   81.59%   +3.32%     
==========================================
  Files           5        5              
  Lines         359      364       +5     
==========================================
+ Hits          281      297      +16     
+ Misses         78       67      -11     
Impacted Files Coverage Δ
src/HSL.jl 58.33% <33.33%> (-27.39%) ⬇️
src/mc21.jl 93.75% <0.00%> (+93.75%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dpo
Copy link
Member

dpo commented Oct 9, 2022

On another note, I don't see the MC21 tests in the output anymore... 🤔

@amontoison
Copy link
Member Author

amontoison commented Oct 9, 2022

Is it possible to test the following code on your computer Dominique?
You don't need the modifications in this PR to run it.

# Current version of HSL
using HSL, MatrixMarket, SuiteSparseMatrixCollection
using LinearAlgebra, Printf, BenchmarkTools

ssmc = ssmc_db(verbose=false)
matrix = ssmc_matrices(ssmc, "Boeing", "pwtk")
path = fetch_ssmc(matrix, format="MM")

n = matrix.nrows[1]
A = MatrixMarket.mmread(joinpath(path[1], "$(matrix.name[1]).mtx"))
b = ones(n)

# Solve Ax = b.
LDL = @btime Ma57($A)
@btime ma57_factorize!($LDL)
@btime ma57_solve($LDL, $b)

blas = "/System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/libBLAS.dylib"
lapack = "/System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/libLAPACK.dylib"
LinearAlgebra.BLAS.lbt_forward(blas, clear=true, verbose=true)
LinearAlgebra.BLAS.lbt_forward(lapack, clear=false, verbose=true)

# Solve Ax = b.
LDL = @btime Ma57($A)
@btime ma57_factorize!($LDL)
@btime ma57_solve($LDL, $b)

@amontoison
Copy link
Member Author

amontoison commented Oct 9, 2022

On another note, I don't see the MC21 tests in the output anymore... thinking

I changed HSL_MC21_PATH into MC21_PATH with #119.
You could use the new variable HSL_ARCHIVES_PATH now.

@JSOBot
Copy link

JSOBot commented Oct 9, 2022

Testing HSL tests passed: https://gist.github.com/f1c96b39f37c7cc4d81f99f1343b9d46

@dpo
Copy link
Member

dpo commented Oct 9, 2022

julia> @btime ma57_factorize($LDL)
ERROR: MethodError: no method matching ma57_factorize(::Ma57{Float64})
Closest candidates are:
  ma57_factorize(::SparseArrays.SparseMatrixCSC{T, Ti}; kwargs...) where {T<:Union{Float32, Float64}, Ti<:Integer} at ~/.julia/packages/HSL/X5ozr/src/hsl_ma57.jl:472

@amontoison
Copy link
Member Author

julia> @btime ma57_factorize($LDL)
ERROR: MethodError: no method matching ma57_factorize(::Ma57{Float64})
Closest candidates are:
  ma57_factorize(::SparseArrays.SparseMatrixCSC{T, Ti}; kwargs...) where {T<:Union{Float32, Float64}, Ti<:Integer} at ~/.julia/packages/HSL/X5ozr/src/hsl_ma57.jl:472

It's ma57_factorize!.

@dpo
Copy link
Member

dpo commented Oct 9, 2022

julia> LDL = @btime Ma57($A)
  499.216 ms (40 allocations: 345.10 MiB)

julia> @btime ma57_factorize!($LDL)
  2.054 s (0 allocations: 0 bytes)

julia> @btime ma57_solve($LDL, $b)
  40.659 ms (4 allocations: 3.33 MiB)

After the BLAS change

julia> LDL = @btime Ma57($A)
  503.579 ms (40 allocations: 345.10 MiB)

julia> @btime ma57_factorize!($LDL)
  1.976 s (0 allocations: 0 bytes)

julia> @btime ma57_solve($LDL, $b)
  45.326 ms (4 allocations: 3.33 MiB)

@dpo
Copy link
Member

dpo commented Oct 9, 2022

@JSOBot runtests

@amontoison
Copy link
Member Author

amontoison commented Oct 9, 2022

julia> LDL = @btime Ma57($A)
  499.216 ms (40 allocations: 345.10 MiB)

julia> @btime ma57_factorize!($LDL)
  2.054 s (0 allocations: 0 bytes)

julia> @btime ma57_solve($LDL, $b)
  40.659 ms (4 allocations: 3.33 MiB)

After the BLAS change

julia> LDL = @btime Ma57($A)
  503.579 ms (40 allocations: 345.10 MiB)

julia> @btime ma57_factorize!($LDL)
  1.976 s (0 allocations: 0 bytes)

julia> @btime ma57_solve($LDL, $b)
  45.326 ms (4 allocations: 3.33 MiB)

OpenBLAS is already efficient 🤔 , we could keep OpenBLAS32 by default and just add in the documentation how to load MKL or Apple Accelerate (#98).

@dpo
Copy link
Member

dpo commented Oct 9, 2022

Yes, that sounds good.

@dpo
Copy link
Member

dpo commented Oct 10, 2022

@JSOBot runtests

@dpo
Copy link
Member

dpo commented Oct 10, 2022

The error with JSOBot is

ERROR: Unsatisfiable requirements detected for package libblastrampoline_jll [8e850b90]:
 libblastrampoline_jll [8e850b90] log:
 ├─libblastrampoline_jll [8e850b90] has no known versions!
 └─restricted to versions * by HSL [34c5aeac] — no versions left
   └─HSL [34c5aeac] log:
     ├─possible versions are: 0.3.2 or uninstalled
     └─HSL [34c5aeac] is fixed to version 0.3.2

It's stuck on Julia 1.6.

@amontoison
Copy link
Member Author

amontoison commented Oct 10, 2022

The error with JSOBot is

ERROR: Unsatisfiable requirements detected for package libblastrampoline_jll [8e850b90]:
 libblastrampoline_jll [8e850b90] log:
 ├─libblastrampoline_jll [8e850b90] has no known versions!
 └─restricted to versions * by HSL [34c5aeac] — no versions left
   └─HSL [34c5aeac] log:
     ├─possible versions are: 0.3.2 or uninstalled
     └─HSL [34c5aeac] is fixed to version 0.3.2

It's stuck on Julia 1.6.

LBT_jll is shipped with Julia, just like OpenBLAS_jll. I don't understand why we have this error. 🤔

@dpo
Copy link
Member

dpo commented Oct 11, 2022

Could you test locally with 1.6?

@amontoison
Copy link
Member Author

Could you test locally with 1.6?

I just tested and it works fine.

@dpo
Copy link
Member

dpo commented Oct 11, 2022

what do we do with this?

@amontoison
Copy link
Member Author

We close it and I will add documentation about LBT (#98)

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.

3 participants