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 generic fallback for Blas.LinAlg.axpy! #5189

Merged
merged 1 commit into from
Dec 19, 2013
Merged

Conversation

stevengj
Copy link
Member

In cases where one wants to use axpy! to reduce memory allocation (see e.g. JuliaLinearAlgebra/IterativeSolvers.jl#16), it is annoying that it is only defined for Arrays of BlasFloat types. This patch provides a fallback for arbitrary array-like and range-like types.

Also, I noticed that axpy!(alpha, x, y) for BLAS types returned y, but axpy!(alpha, x, rx, y, ry) returned a pointer to data in y. The latter exposure of the internal implementation seems undesirable and not very useful, so I modified it to return y.

@andreasnoack
Copy link
Member

You have really turned your back to specifying types. Could there be a potential problem with difficult error messages when providing illegal inputs?

Besides that I think it looks good except that I think it belongs in generic.jl.

@stevengj
Copy link
Member Author

It's hard to break the habit of specifying types, but I'm beginning to feel that in Julia you should have a concrete reason to declare a type rather than vice versa. But if people feel strongly I about this case I could sprinkle a few AbstractArray declarations in there.

If we put it in generic.jl then we should probably export axpy! from LinAlg, not just from LinAlg.BLAS. This seems reasonable to me, but the counter-argument is that axpy! is very BLASy (BLASé?) flavored.

@jiahao
Copy link
Member

jiahao commented Dec 18, 2013

I'm fine the way it is. It's just a fallback.

I don't have a strong opinion on blas.jl vs generic.jl. it depends on whether blas.jl should only contain blas wrappers.

@simonster
Copy link
Member

It may be worth specifying types for rx and ry. Otherwise if someone forgets n for axpy!(n::Integer,alpha::Complex{Float64},dx::Union(Ptr{Complex{Float64}},Array{Complex{Float64},N}),incx::Integer,dy::Union(Ptr{Complex{Float64}},Array{Complex{Float64},N}),incy::Integer) this method will get called instead and silently give different behavior.

Is it worth considering @inbounds?

@stevengj
Copy link
Member Author

Moved to generic.jl in LinAlg, added a few AbstractArray type declarations (this routine seems unlikely to be used with non-AbstractArray types in practice), and used inbounds.

jiahao added a commit that referenced this pull request Dec 19, 2013
add generic fallback for Blas.LinAlg.axpy!
@jiahao jiahao merged commit 0d499cf into JuliaLang:master Dec 19, 2013
@jiahao
Copy link
Member

jiahao commented Dec 19, 2013

With this and Givens, it's getting to the point where we'll have to make IterativeSolvers post-0.2.0-compatible only.

@StefanKarpinski
Copy link
Member

You can always just do

if VERSION < v"0.3"
  # define things that weren't in 0.2 yet
end

It's a bit of an eyesore, but it's easy to isolate and easy to delete later when it's no longer needed.

@jiahao
Copy link
Member

jiahao commented Dec 19, 2013

That's almost above my pay grade to care about.

@stevengj
Copy link
Member Author

Who uses 0.2 anymore? Any Julia version more than two days old is obviously a fossil.

@StefanKarpinski
Copy link
Member

0.2 is so last month.

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