Implement gesdd
#899
Implement gesdd
#899
Conversation
jmachado-amd
added
the
noOptimizations
jmachado-amd
requested review from
jzuniga-amd,
tfalders,
cgmb,
qjojo,
EdDAzevedo,
AGonzales-amd and
a team
as code owners
|
|
||
| for(j = tid; j < n; j += gridDim.x * blockDim.x) | ||
| { | ||
| for(k = 0; k < n; k++) |
There was a problem hiding this comment.
I wonder whether the code can use more threads or more parallelism, say batch index b in z-dimension, j in x-dimension (as inner loop) , k in y-dimension. Just a thought.
There was a problem hiding this comment.
It can be improved, but this is intended as an intermediate step and not the final algorithm.
| size_t* size_workArr2) | ||
| { | ||
| // If quick return, set workspace to zero | ||
| if(n == 0 || m == 0 || batch_count == 0) |
There was a problem hiding this comment.
minor comment: perhaps set all sizes to zero first, then check if (n==0) and return. This is just defensive programming to make sure all the size variables are initialized, no undefined values. Just a minor suggestion.
| *size_splits = std::max({f1}); | ||
| *size_tmptau_W = std::max({g1}); | ||
| *size_tau = std::max({h1}); | ||
| *size_workArr = sizeof(T) * std::max({m, n}) * std::max({m, n}) * batch_count; |
There was a problem hiding this comment.
Just double checking whether the size_workArr is max(m,n)^2 * batch_count. It might be a very big number.
There was a problem hiding this comment.
Yes, this looks like a typo, and I'll check it later. Thanks for catching that!
There was a problem hiding this comment.
As it turns out, that code was semantically incorrect: it was supposed to reserve space for the off-diagonal elements of the input matrix, as required by stedc. Thanks again for catching it!
|
@jmachado-amd : Would this feature require a changelog update? If you're planning on adding that as a separate PR, then no worries. |
|
@EdDAzevedo, my opinion is that svd algorithms based on creating the explicit products |
This PR adds a minimal implementation of
gesdd, usingsyevdas a backend. A complete implementation, usingbdsdc, will appear in the future.