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

oneapi::mkl::lapack::getrf pivot vectors are wrong #79

Closed
pghysels opened this issue Feb 26, 2021 · 7 comments
Closed

oneapi::mkl::lapack::getrf pivot vectors are wrong #79

pghysels opened this issue Feb 26, 2021 · 7 comments
Labels
3rd party The issue/request was root caused to 3d party implementation and cannot be addressed in oneAPI

Comments

@pghysels
Copy link

Summary

The oneapi::mkl::lapack::getrf routine takes a std::int64_t pointer for the pivots.
But the output for the pivots seems wrong. It looks like they are generated as 32 bit numbers.
The corresponding getrs code works with the output from oneapi::mkl::lapack::getrf.

Version

oneapi/2020.12.15.005

Environment

  • Intel(R) Xeon(R) Gold 6226R CPU @ 2.90GHz

Steps to reproduce

#include <iostream>
#include <CL/sycl.hpp>
#include "oneapi/mkl.hpp"

int main(int argc, char* argv[]) {
  cl::sycl::queue q(cl::sycl::host_selector{});
  int n = 10;
  std::vector<double> A(n*n);
  for (int i=0; i<n; i++) A[i+i*n] = 1.;
  auto iwork = oneapi::mkl::lapack::getrf_scratchpad_size<double>(q, n, n, n);
  std::vector<double> work(iwork);
  std::vector<std::int64_t> piv(n);
  oneapi::mkl::lapack::getrf(q, n, n, A.data(), n, piv.data(), work.data(), iwork);
  for (auto p : piv) std::cout << p << " ";
  std::cout << std::endl;
  for (int i=0; i<n; i++) std::cout << reinterpret_cast<int*>(piv.data())[i] << " ";
  std::cout << std::endl;
}

Observed behavior

Output of above code:
8589934593 17179869187 25769803781 34359738375 42949672969 0 0 0 0 0
1 2 3 4 5 6 7 8 9 10

Expected behavior

Document behavior you expect.
1 2 3 4 5 6 7 8 9 10

@sknepper
Copy link
Contributor

Thank you for the detailed bug report, including a reproducer! This project doesn't cover oneMKL binary product and the better way to escalate issues is the oneMKL Forum. You can read more about difference between oneMKL interfaces project and Intel oneMKL release here.

That said, the Intel oneMKL team will take a look at this issue now that you've brought it to our attention. Thanks!

@sknepper
Copy link
Contributor

We took a look, and it seems like the link line may be incorrect. Please consider using the Link Line Advisor to configure the link command according to your program features. In particular, Intel® oneAPI Math Kernel Library for DPC++ only supports using the mkl_intel_ilp64 interface library.

Linking with the mkl_intel_ilp64 interface library and defining the compilation option -DMKL_ILP64 allow the program to run with the expected behavior:
1 2 3 4 5 6 7 8 9 10
1 0 2 0 3 0 4 0 5 0

While linking to the mkl_intel_lp64 interface library resulted in the same output you reported.

Example link line:
dpcpp dgetrf.cpp -DMKL_ILP64 -I${MKLROOT}/include -L${MKLROOT}/lib/intel64 -lmkl_sycl -lmkl_intel_ilp64 -lmkl_tbb_thread -lmkl_core -lsycl -lOpenCL -lpthread -lm -ldl

Thanks, and please let us know if this doesn't solve the issue.

@pghysels
Copy link
Author

Hi. Thank you. That does indeed solve the issue.

@pghysels
Copy link
Author

I'm sorry I didn't show my link line in the original post.
I have used the link line advisor before, it is a great tool.

In other parts of my code I use the original 32 bit fortran BLAS and LAPACK interfaces directly. I wonder how I will be able to combine both these 32 bit interfaces and the 64 bit oneAPI interfaces in a single library.

@mkrainiuk
Copy link
Contributor

Hi @pghysels, so far it's not possible, but we will investigate how we can enable mixing of lp64/ilp64 APIs in one library. Is it important to have 32 bit fortran API in your application?

@pghysels
Copy link
Author

pghysels commented Mar 1, 2021

The 32 bit Fortran interface is used extensively in our code. I could refactor this to make the BLAS/LAPACK integer size a template parameter or make it configurable at compile time. But this would require considerable changes. We didn't think we would ever need 64 bit LAPACK.
This 32 bit interface is also used in some of our dependencies, for instance SLATE, (which does not currently support MKL with 64 bit integers) and ButterflyPACK, which is written in Fortran and calls the 32 bit interfaces directly.

@mkrainiuk
Copy link
Contributor

Thanks @pghysels, I will keep this issue updated with any information about enabling lp64/ilp64 mixing support in Intel oneMKL.

@jasukhar jasukhar added the 3rd party The issue/request was root caused to 3d party implementation and cannot be addressed in oneAPI label Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party The issue/request was root caused to 3d party implementation and cannot be addressed in oneAPI
Projects
None yet
Development

No branches or pull requests

4 participants