search

gonum / blas

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

Incorrect doc comment for Dgemm #120

Closed btracey closed 7 years ago

btracey commented 9 years ago

Comment says that A, B, and C are mxn, but this isn't true. C is always m x n, but A and B are never m x n.

kortschak commented 9 years ago

The docs previously said nxn which is not correct. The meaning I had in the comment was that they are general nxm matrices (as opposed to nxn square matrices). I considered explicitly stating the sizes, but thought the comment would become unwieldy, particularly when the transpose state is included in the mix.

For comparison, the relevant part of the relevant dgemm.f comment:

M specifies the number of rows of the matrix op(A) and of the matrix C.

N specifies the number of columns of the matrix op(B) and the number of columns of the matrix C.

K specifies the number of columns of the matrix op(A) and the number of rows of the matrix op(B).

If we didn't have to mention the transpose operations and we want the specific dimensions, it would be "A, B, and C are m×k, k×n and m×n dense matrices respectively."

So maybe make it "op(A), op(B), and C are m×k, k×n and m×n dense matrices respectively, where op is transpose or identity." I'm not wildly happy about the wording.

kortschak commented 8 years ago

This issue is stale; @vladimir-ch made changes to the comment that invalidate this issue. Is it still a concern or can we close this?