Gemv degen fix #568
Gemv degen fix #568
Conversation
gemv with degenerate matrices in transpose mode return incorrect results. Previously detected with cublas, but also observed with host blas.
src/blas/KokkosBlas2_gemv.hpp
Outdated
| && !std::is_same<typename AViewType::device_type::execution_space, Kokkos::Cuda>::value | ||
| #endif | ||
| ) | ||
| if ((trans[0] == 'T' || trans[0] == 't' || trans[0] == 'C' || trans[0] == 'c' || trans[0] == 'H' || trans[0] == 'h')) |
There was a problem hiding this comment.
Why "h" ? T is transpose and C is conj transpose. What does H mean ?
There was a problem hiding this comment.
Fixed with new push
| @@ -139,7 +139,7 @@ gemv (const char trans[], | |||
| typename YViewType::device_type, | |||
| Kokkos::MemoryTraits<Kokkos::Unmanaged> > YVT; | |||
|
|
|||
| if ((trans[0] == 'T' || trans[0] == 't' || trans[0] == 'C' || trans[0] == 'c' || trans[0] == 'H' || trans[0] == 'h')) | |||
| if ((trans[0] == 'T' || trans[0] == 't' || trans[0] == 'C' || trans[0] == 'c') && A.extent(0) == 0) | |||
There was a problem hiding this comment.
I still don't understand the context. A is m x n matrix and m is zero. The spec said y = beta y + alpha A x. So, in this case, y = beta y + alpha A' x. If A is degenerated, then this is scaling y = beta y. How come this makes a special case that is distinct from non transpose case ? Also, why do we only consider degeneration case for m = 0 ? When n (extent(1)) is zero, this should behave the same.
There was a problem hiding this comment.
If A is degenerated, then this is scaling y = beta y
Correct. When testing the degenerated case with transpose mode with m = 0 the gemv result (which essentially acts as scaling like you pointed out) results in incorrect results. This occurs when tpls are enabled; short-circuiting the code path to instead use the fallback implementation allows for correct results (via passing tests).
Part of the reason this was not showing up prior is because transpose mode was not being tested, Ichi added this when he updated the fallback implementation for transpose mode.
There was a problem hiding this comment.
@kyungjoo-kim issue #539 has discussion of when this problem first observed.
|
@kyungjoo-kim I'll close this PR and put in a new one where |
No description provided.