gonum / blas

A BLAS implementation for Go [DEPRECATED]
172 stars 16 forks source link

Fix Dgemv when incX == 1 but incY != 1. Fix minor comment while here. #119

Closed btracey closed 9 years ago

kortschak commented 9 years ago

Whoops.

LGTM after float32 code has been generated (maybe a test as well).

btracey commented 9 years ago

I ran go generate, but it did not have the desired result Before:

// Sgemv computes
//  y = alpha * a * x + beta * y if tA = blas.NoTrans
//  y = alpha * A^T * x + beta * y if tA = blas.Trans or blas.ConjTrans
// where A is an m×n dense matrix, x and y are vectors, and alpha is a scalar.
//
// Float32 implementations are autogenerated and not directly tested.
func (Implementation) Sgemv(tA blas.Transpose, m, n int, alpha float32, a []float32, lda int, x []float32, incX int, beta float32, y []float32, incY int) {
    if tA != blas.NoTrans && tA != blas.Trans && tA != blas.ConjTrans {
        panic(badTranspose)
    }

After

// Sgemv computes
//  y = alpha * a * x + beta * y if tA = blas.NoTrans
//  y = alpha * A^T * x + beta * y if tA = blas.Trans or blas.ConjTrans
// where A is an m×n dense matrix, x and y are vectors, and alpha is a scalar.
//n// Float32 implementations are autogenerated and not directly tested.nfunc (Implementation) Sgemv(tA blas.Transpose, m, n int, alpha float32, a []float32, lda int, x []float32, incX int, beta float32, y []float32, incY int) {
    if tA != blas.NoTrans && tA != blas.Trans && tA != blas.ConjTrans {
        panic(badTranspose)

It's clear that there are a couple of newline errors, but I'm having trouble parsing the sed command to see how it needs to be changed.

kortschak commented 9 years ago

Try replacing the single quotes wrapping the sed expressions. I'm guessing this is a difference between GNU sed and BSD sed.

btracey commented 9 years ago

I removed the single quotes and it didn't work.

brendan:~/Documents/mygo/src/github.com/gonum/blas/native$ go generate
Generating level1single.go
./single_precision: line 22: syntax error near unexpected token `('
./single_precision: line 22: `| sed -e s_^\(func (Implementation) \)D\(.*\)$_//\n// Float32 implementations are autogenerated and not directly tested.\n\1S\2_ \'

When replacing the first set with quotations (") I get:

brendan:~/Documents/mygo/src/github.com/gonum/blas/native$ go generate
Generating level1single.go
sed: 1: "s_^\(func (Implementati ...": unescaped newline inside substitute pattern

Does that provide the hint?

kortschak commented 9 years ago

I'll have a look at sed on one of my students' machines later today. It hints atnwhat I've long suspected: osx is not an os I want to use.

btracey commented 9 years ago

I made an identical change to Single (by hand) and added a test. Can I submit the change since it fixes incorrect behavior? We can fix the "go generate" script next.

kortschak commented 9 years ago

Please do.

kortschak commented 9 years ago

For the generate fix: http://nlfiedler.github.io/2010/12/05/newlines-in-sed-on-mac.html

For the life of me, I don't understand why people still use these machines.