Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KokkosBlas::dot is broken for complex, due to incorrect assumptions about Fortran ABI #307

Closed
mhoemmen opened this issue Oct 1, 2018 · 9 comments

Comments

@mhoemmen
Copy link
Contributor

mhoemmen commented Oct 1, 2018

@kyungjoo-kim @crtrott See comment here: trilinos/Trilinos#3493 (comment)

@mhoemmen
Copy link
Contributor Author

mhoemmen commented Oct 1, 2018

See this article on the relevant functions: http://mirror.informatimago.com/next/developer.apple.com/hardware/ve/errata.html

@mhoemmen
Copy link
Contributor Author

mhoemmen commented Oct 1, 2018

https://www.math.utah.edu/software/c-with-fortran.html#function-return-types

"...you should not expect to use Fortran functions that return types such as COMPLEX or COMPLEX*16. Write a SUBROUTINE interface to your Fortran function instead, and then invoke it as a void function from C or C++."

mhoemmen pushed a commit to mhoemmen/Trilinos that referenced this issue Oct 1, 2018
@trilinos/tpetra

Patch Trilinos' current snapshot of kokkos-kernels, in order to
fix trilinos#3493 and kokkos/kokkos-kernels#307.
@mhoemmen
Copy link
Contributor Author

mhoemmen commented Oct 1, 2018

My Trilinos PR trilinos/Trilinos#3538 only minimally patches kokkos-kernels to fix this issue. I leave it to the kokkos-kernels developers to figure out the best longer-term approach. However, please do fix this before the next develop-to-master promotion.

@srajama1
Copy link
Contributor

srajama1 commented Oct 2, 2018

@kyungjoo-kim Can you take care of this please ?

@kyungjoo-kim
Copy link
Contributor

@crtrott In your dot testing, why do you use the following views ?

    typedef Kokkos::View<ScalarA*[2],                                                                                                                        
       typename std::conditional<                                                                                                                            
                std::is_same<typename ViewTypeA::array_layout,Kokkos::LayoutStride>::value,                                                                  
                Kokkos::LayoutRight, Kokkos::LayoutLeft>::type,Device> BaseTypeA;                                                                            
    BaseTypeA b_a("A",N);                                                                                                                                    
    ViewTypeA a = Kokkos::subview(b_a,Kokkos::ALL(),0);                                                                                                      

In this way, the view is strided and it never tests any tpls as tpls require unit stride according to your ETI design.

@mhoemmen
Copy link
Contributor Author

mhoemmen commented Oct 4, 2018

@kyungjoo-kim _DOT in the BLAS takes strides (INCX and INCY).

@kyungjoo-kim
Copy link
Contributor

@mhoemmen I know that the standard blas interface has the stride interface but kokkoskernels ETI system instanciates for layoutleft (using unification of layout for 1D view layout right and layout left goes into the instanciated code but the strided case uses kokkos native implementation).

@kyungjoo-kim
Copy link
Contributor

@crtrott I confused view semantics and thought subview will return stride view. It is all oaky.

@ndellingwood
Copy link
Contributor

Should be addressed by PR #314, thanks @kyungjoo-kim and @mhoemmen !

@crtrott crtrott closed this as completed Nov 5, 2018
tjfulle pushed a commit to tjfulle/Trilinos that referenced this issue Dec 6, 2018
@trilinos/tpetra

Patch Trilinos' current snapshot of kokkos-kernels, in order to
fix trilinos#3493 and kokkos/kokkos-kernels#307.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants