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

Dense linear algebra cleanups #2062

Closed
7 tasks
ViralBShah opened this issue Jan 16, 2013 · 13 comments
Closed
7 tasks

Dense linear algebra cleanups #2062

ViralBShah opened this issue Jan 16, 2013 · 13 comments
Milestone

Comments

@ViralBShah
Copy link
Member

The proposal is to

  • Replace all linear algebra errors with exceptions. Also, avoid names like LAPACK.LapackDimMisMatch where lapack is repeated twice.
  • In blas.jl, avoid repetition of all functions twice. For eg., make gemm call gemm!.
  • Add Factorization and related methods to documentation
  • For svd perhaps we should have a Factorization type as well that makes it easy to solve
  • Rename chold to cholfact etc. for consistency
  • Add more convenience methods to blas.jl with default values for parameters such as alpha and beta in dgemm.
  • Move constructors out of the type block to make them generally available.

[ edit: Add a few more work items. Keep the ! versions also -- ViralBShah]

@mlubin
Copy link
Member

mlubin commented Jan 16, 2013

Why drop the ! version of chold?

@dmbates
Copy link
Member

dmbates commented Jan 16, 2013

The ! version of the LAPACK.potrf! function will still be available for programmers who wish to overwrite an existing array. Having chold! in addition seemed like overkill.

@mlubin
Copy link
Member

mlubin commented Jan 16, 2013

Unless I misunderstand, it seems like that removes the (useful) functionality of getting a Factorization object when you want to avoid copying the input matrix. What about things like UmfpackLU!?

@Keno
Copy link
Member

Keno commented Jan 16, 2013

Maybe have chold do the usual copying thing and have the constructors take ownership of the matrix and possibly overwrite them?

@dmbates
Copy link
Member

dmbates commented Jan 17, 2013

@ViralBShah Do you plan to create a branch for working on these changes?

@ViralBShah
Copy link
Member Author

I have created vs/linalg and will also make it an in-progress pull-request.

@ViralBShah
Copy link
Member Author

@mlubin We will lose that capability for dense in routines such as chold!. I feel that chold! is a bit of an in-between thing in that it tries to provide the generality of the LAPACK interface, but fallls short. In the process also trashes the input, while returning a new Factorization type that embeds that input, which may not be obvious until you dig into it. Thus, it seems that we should have a user-friendly interface, where chol returns matrices from the factorization, cholfact (new name for chold) returns a Factorization object for re-using multiple times. For all other uses, the LAPACK interface should be used, and it already does good things like figure out dimensions, work arrays and all that.

Depending on how the dense stuff goes, we will also need to rethink the sparse stuff. The interfaces need an overhaul in any case in sparse due to the recent USE_LIB64 change, which compiles all sparse libraries in 64-bit mode.

@dmbates
Copy link
Member

dmbates commented Jan 17, 2013

Shall we create a

type DimMisMatchException <: Exception

in base.jl? I want to throw such an exception in blas.jl and it probably wouldn't be a good idea to have the BLAS module import the LAPACK module.

@ViralBShah
Copy link
Member Author

That seems like a good idea, so that we can give more structured errors in a bunch of other places as well.

@mlubin
Copy link
Member

mlubin commented Jan 17, 2013

I think the operation of factorizing a matrix and destroying the input is a reasonably high-level concept that someone who doesn't know anything about LAPACK or potrf might want to do. I don't see why it should be necessary to drop down to a lower-level interface for this.

@StefanKarpinski
Copy link
Member

I'm kind of with Miles on this one. Being able to avoid allocating temporary arrays without needing to learn the rather esoteric and complex LAPACK interface seems like a good thing to me. I'd rather read a sentence about how chold! or cholfact! works than figure out a) that I need to look at LAPACK.potrf and then read about how in the world that function works.

@ViralBShah
Copy link
Member Author

Let's do all the other work here and keep the bang versions of these functions as they exist, until we have a clearer idea.

ViralBShah added a commit that referenced this issue Feb 4, 2013
Relevant comments are in #2062.
@StefanKarpinski
Copy link
Member

I'm going to move this from 0.1 to 0.2 also.

ViralBShah added a commit that referenced this issue Feb 9, 2013
  commit 5c1e646
    Chain methods for gemm and gemv to gemm! and gemv!
    Add gemm and gemv methods that have an implicit 1.0 multiplier
  commit 11c8f36
    Chain more foo methods to foo!, add blas tests.
    Reformat the linalg tests.

Relevant comments are in #2062.

Conflicts:
	test/Makefile
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

No branches or pull requests

5 participants