Merge blas convience wrapper with blas

[dane-unltd]

In my opinion it would be reasonable to have this stuff in one place

[kortschak]

I'm ambivalent about the location, so I'd like to wait for other input on that.

Can we make the layout of General the same as mat64.RawMatrix please; that layout may be required by clients and it's been around for a while.

[btracey]

I’m mostly ambivalent about the location of this package with one exception. It seems to me that once we have a full go blas implementation (even for just float64) we would like to make it the default. This way people who want a custom blas implementation can have it, but people who don’t care can use the matrix package without any extra work. Right now, this is impossible, because blas would need to import goblas to make it the default, but goblas needs to import blas in order to implement the interface.

It seems like we should allow calls to the blas implementation directly without forcing the use of the convenience wrapper. This would allow a package author to circumvent the Vector and Matrix types if necessary (for some reason), while also allowing users of the package to customize their blas engine.

I don’t think we need most of the checks that are present in the code. The standard we are using already defines that the blas routines should panic. It doesn’t seem like there’s any sense in adding another layer of them. There are exceptions, for one, the netlib standard says that negative indices are not allowed for level1 routines that only contain one vector argument, but the behavior is to return without error. This seems like unfortunate behavior, and so adding that check in this convenience wrapper would make sense. Similarly, we need to add a check for the order of the matrices.

On Jan 28, 2014, at 2:15 PM, kortschak notifications@github.com wrote:

I'm ambivalent about the location, so I'd like to wait for other input on that.

Can we make the layout of General the same as mat64.RawMatrix please; that layout may be required by clients and it's been around for a while.

— Reply to this email directly or view it on GitHub.

[dane-unltd]

I think i will remove the checks for single variables from the function calls (i.e. A.Check() etc.) and only check for matching dimensions if there are several vector/matrix input arguments

[kortschak]

If gonum/blas does not say that it panics when there are dimension mismatches, that is an error and I will fix it. You should not need to make any dimension checks - mat64 does, but that is to hide implementation details from the client.

[kortschak]

Sometimes I should just stay quiet. Carry on.

[dane-unltd]

From #2: what about naming the wrapper packages blasd, blasz, blass, blasc? We could also remove the first letters from the blas functions in the wrappers (the numerical type moved to the package name) blasd.Axpy(...), blasz.Axpy(...) etc.

[btracey]

if we’re going with the five letter names, it should be dblas, zblas, etc.

I don’t think we should remove the leading letter. It seems like it’ll be more confusing for people used to using blas, and one extra letter isn’t that big of a deal.

On Jan 31, 2014, at 8:38 AM, dane-unltd notifications@github.com wrote:

From #2: what about naming the wrapper packages blasd, blasz, blass, blasc? We could also remove the first letters from the blas functions in the wrappers (the numerical type moved to the package name) blasd.Axpy(...), blasz.Axpy(...) etc.

— Reply to this email directly or view it on GitHub.

[dane-unltd]

Right, that is less ugly, but then we have a collision with cblas. Maybe we can rename the cblas package? and while we are at it maybe remove the blas suffix from referenceblas and testblas?

[dane-unltd]

The argument for removing the type indicating letters from the blas functions would be, that it is easier to switch e.g. from 64 to 32 bit because you can just change import bw "github.com/gonum/blas/dblas" into import bw "github.com/gonum/blas/sblas"

[kortschak]

We could borrow from Jan's book and have very short names: blas/[sdcz]?

The removal of suffixes sounds good.

I'm not convinced by the argument about drop-in replacement for different numerical types. People probably shouldn't be doing that and if they really want to gofmt/sed will work for them.

[dane-unltd]

For blas level 2, if we use methods on the matrices instead of functions we can also drop the matrix type prefix, which could actually make it possible to declare interfaces for matrix-vector multiplication and for vector solve.

[kortschak]

This set of changes is deviating significantly from a blas convenience wrapper. There are changes here that I'm interested in, but the level of change is too great and unfortunately there is no fine grained control of PR merging on github. If these changes are to be merged they need to be cherry picked.

On the issue of making these things methods. I'm not sure yet. I'd like to see how the wrapper API looks before we move on other changes. Having said that, if the routines are provided as methods, then it should be done consistently; level 1 should be methods on vectors and level 3 should also be methods on matrices. The latter portion makes the wrapper look like a competitor for matrix (i.e. a non-orthogonal addition) - this may be something we want, but it needs to be discussed more.

[dane-unltd]

I agree that there should be some discussion. I will leave the branch in the current state until we made a decision on the API, then I can revert/finish up and merge it in.

For level 1 there is not much of a use case for interfaces so changes would be purely aesthetic. It is straightforward for the cases with 1 vector argument, but methods for the functions with 2 vector arguments would be a bit awkward in my opinion.

The level 3 case could be done analogously to the level 2 case, i.e., moving the prefix defining argument to the front.

Well, since parts of the matrix package is also wrapping the blas functionality, there is some overlap. But changing from function to method signatures does not change anything in the functionality of the wrapper, so any non-orthogonality will be there either way.

[kortschak]

If there is a difficulty with two vector routines then there is a difficulty with two matrix routines. I'm feeling increasingly uneasy about this.

[dane-unltd]

I reverted back to function definitions for now. We can always add some methods later on (additional to the functions) if there is a use case.

[dane-unltd]

So, to merge or not to merge? Maybe we could just do it since the additions are not connected to anything else in the repository. Then we can do some experimentations with it in other packages.

[kortschak]

I'll have a look today.

[dane-unltd]

I realized that the short package names (d, z) do not really work, since one letter variables appear frequently in numerical code. Maybe [d,z,s,c]bw ?

[kortschak]

That looks fine to me with those name changes.