[GitHub] asmushetzel commented on a change in pull request #8577: support for lapack functions with mkl

[GitBox]

asmushetzel commented on a change in pull request #8577: support for lapack 
functions with mkl
URL: https://github.com/apache/incubator-mxnet/pull/8577#discussion_r150193519
 
 

 ##
 File path: src/operator/c_lapack_api.h
 ##
 @@ -160,7 +162,75 @@ inline void flip(int m, int n,
 }
 
 
-#if MXNET_USE_LAPACK
+#if (MSHADOW_USE_MKL && MXNET_USE_LAPACK)
+
+  // We interface with the C-interface of MKL
+  // as this is the preferred way.
+  #include 
+
+  #define MXNET_LAPACK_ROW_MAJOR LAPACK_ROW_MAJOR
+  #define MXNET_LAPACK_COL_MAJOR LAPACK_COL_MAJOR
+
+  // These function have already matching signature.
+  #define MXNET_LAPACK_spotrf LAPACKE_spotrf
+  #define MXNET_LAPACK_dpotrf LAPACKE_dpotrf
+  #define MXNET_LAPACK_spotri LAPACKE_spotri
+  #define MXNET_LAPACK_dpotri LAPACKE_dpotri
+  #define mxnet_lapack_sposv  LAPACKE_sposv
+  #define mxnet_lapack_dposv  LAPACKE_dposv
+
+  // The following functions differ in signature from the
+  // MXNET_LAPACK-signature and have to be wrapped.
+  #define MXNET_LAPACK_CWRAP_GELQF(prefix, dtype) \
+  inline int MXNET_LAPACK_##prefix##gelqf(int matrix_layout, int m, int n, \
+  dtype *a, int lda, dtype* tau, \
+  dtype* work, int lwork) { \
+if (lwork != -1) { \
+  return LAPACKE_##prefix##gelqf(matrix_layout, m, n, a, lda, tau); \
+} \
+*work = 0; \
+return 0; \
+  }
+  MXNET_LAPACK_CWRAP_GELQF(s, float)
+  MXNET_LAPACK_CWRAP_GELQF(d, double)
+
+  #define MXNET_LAPACK_CWRAP_ORGLQ(prefix, dtype) \
+  inline int MXNET_LAPACK_##prefix##orglq(int matrix_layout, int m, int n, \
+  dtype *a, int lda, dtype* tau, \
+  dtype* work, int lwork) { \
+if (lwork != -1) { \
+  return LAPACKE_##prefix##orglq(matrix_layout, m, n, m, a, lda, tau); \
+} \
+*work = 0; \
+return 0; \
+  }
+  MXNET_LAPACK_CWRAP_ORGLQ(s, float)
+  MXNET_LAPACK_CWRAP_ORGLQ(d, double)
+
+  // This has to be called internally in COL_MAJOR format even when 
matrix_layout
+  // is row-major as otherwise the eigenvectors would be returned as cols in a
+  // row-major matrix layout (see MKL documentation).
+  // We also have to allocate at least one DType element as workspace as the
+  // calling code assumes that the workspace has at least that size.
+  #define MXNET_LAPACK_CWRAP_SYEVD(prefix, dtype) \
+  inline int MXNET_LAPACK_##prefix##syevd(int matrix_layout, char uplo, int n, 
dtype *a, \
 
 Review comment:
   Yes it is a function name. We could argue that we ant to use lower case, on 
the other hand using capital MXNET_LAPACK as a prefix for anything lapack 
related was already introduced longer time ago, so this just adheres to this 
convention. Are you suggesting to change all the function definitions/calls to 
lowercase prefixes?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] asmushetzel commented on a change in pull request #8577: support for lapack functions with mkl

[GitBox]

asmushetzel commented on a change in pull request #8577: support for lapack 
functions with mkl
URL: https://github.com/apache/incubator-mxnet/pull/8577#discussion_r149604263
 
 

 ##
 File path: src/operator/c_lapack_api.h
 ##
 @@ -160,7 +162,73 @@ inline void flip(int m, int n,
 }
 
 
-#if MXNET_USE_LAPACK
+#if (MSHADOW_USE_MKL && MXNET_USE_LAPACK)
+
+  // We interface with the C-interface of MKL 
+  // as this is the preferred way. 
+  #include 
+
+  #define MXNET_LAPACK_ROW_MAJOR LAPACK_ROW_MAJOR
+  #define MXNET_LAPACK_COL_MAJOR LAPACK_COL_MAJOR
+  #define MXNET_LAPACK_spotrf LAPACKE_spotrf 
+  #define MXNET_LAPACK_dpotrf LAPACKE_dpotrf 
+  #define MXNET_LAPACK_spotri LAPACKE_spotri 
+  #define MXNET_LAPACK_dpotri LAPACKE_dpotri 
+  #define mxnet_lapack_sposv  LAPACKE_sposv 
+  #define mxnet_lapack_dposv  LAPACKE_dposv 
+
+  // The following functions differ in signature from the 
+  // MXNET_LAPACK-signature and have to be wrapped.
+  #define MXNET_LAPACK_CWRAP_GELQF(prefix, dtype) \
+  inline int MXNET_LAPACK_##prefix##gelqf(int matrix_layout, int m, int n, \
+  dtype *a, int lda, dtype* tau, \
+  dtype* work, int lwork) { \
+if (lwork != -1) { \
+  return LAPACKE_##prefix##gelqf(matrix_layout, m, n, a, lda, tau); \
+} \
+*work = 0; \
+return 0; \
+  } 
+  MXNET_LAPACK_CWRAP_GELQF(s, float)
+  MXNET_LAPACK_CWRAP_GELQF(d, double)
+
+  #define MXNET_LAPACK_CWRAP_ORGLQ(prefix, dtype) \
+  inline int MXNET_LAPACK_##prefix##orglq(int matrix_layout, int m, int n, \
+  dtype *a, int lda, dtype* tau, \
+  dtype* work, int lwork) { \
+if (lwork != -1) { \
+  return LAPACKE_##prefix##orglq(matrix_layout, m, n, m, a, lda, tau); \
+} \
+*work = 0; \
+return 0; \
+  } 
+  MXNET_LAPACK_CWRAP_ORGLQ(s, float)
+  MXNET_LAPACK_CWRAP_ORGLQ(d, double)
+
+  // This has to be called internally in COL_MAJOR format even when 
matrix_layout
+  // is row-major as otherwise the eigenvectors would be returned as cols in a 
+  // row-major matrix layout (see MKL documentation). 
+  // We also have to allocate at least one DType element as workspace as the 
+  // calling code assumes that the workspace has at least that size. 
+  #define MXNET_LAPACK_CWRAP_SYEVD(prefix, dtype) \
+  inline int MXNET_LAPACK_##prefix##syevd(int matrix_layout, char uplo, int n, 
dtype *a, \
+  int lda, dtype *w, dtype *work, int 
lwork, \
+  int *iwork, int liwork) { \
+if (lwork != -1) { \
+  char o(loup(uplo, (matrix_layout == MXNET_LAPACK_ROW_MAJOR))); \
+  return LAPACKE_##prefix##syevd(LAPACK_COL_MAJOR, 'V', o, n, a, lda, w); \
 
 Review comment:
   formally not needed. just here as MKL has this funny behaviour on output the 
the columns of the matrix are always the eigenvetors, regardless of matrix 
layout. 
   Only needed if we think that the real intel MKL version is internally build 
doing unnecessary transpose-operations when matrix layout is row-major. There 
are lapacke-headers on github that seem to indicate that but  I don't think 
that Intel will put such inefficiencies in a final release of there claimed to 
be most efficient math-kernel-library. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] asmushetzel commented on a change in pull request #8577: support for lapack functions with mkl

[GitBox]

asmushetzel commented on a change in pull request #8577: support for lapack 
functions with mkl
URL: https://github.com/apache/incubator-mxnet/pull/8577#discussion_r149603534
 
 

 ##
 File path: src/operator/c_lapack_api.h
 ##
 @@ -271,6 +339,9 @@ inline void flip(int m, int n,
  " Ensure that lapack library is installed and build with USE_LAPACK=1 to 
get lapack" \
  " functionalities.")
 
+  #define MXNET_LAPACK_ROW_MAJOR 101
 
 Review comment:
   yep


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] asmushetzel commented on a change in pull request #8577: support for lapack functions with mkl

[GitBox]

asmushetzel commented on a change in pull request #8577: support for lapack 
functions with mkl
URL: https://github.com/apache/incubator-mxnet/pull/8577#discussion_r149603492
 
 

 ##
 File path: src/operator/c_lapack_api.h
 ##
 @@ -160,7 +162,73 @@ inline void flip(int m, int n,
 }
 
 
-#if MXNET_USE_LAPACK
+#if (MSHADOW_USE_MKL && MXNET_USE_LAPACK)
+
+  // We interface with the C-interface of MKL 
+  // as this is the preferred way. 
+  #include 
+
+  #define MXNET_LAPACK_ROW_MAJOR LAPACK_ROW_MAJOR
+  #define MXNET_LAPACK_COL_MAJOR LAPACK_COL_MAJOR
+  #define MXNET_LAPACK_spotrf LAPACKE_spotrf 
+  #define MXNET_LAPACK_dpotrf LAPACKE_dpotrf 
+  #define MXNET_LAPACK_spotri LAPACKE_spotri 
+  #define MXNET_LAPACK_dpotri LAPACKE_dpotri 
+  #define mxnet_lapack_sposv  LAPACKE_sposv 
+  #define mxnet_lapack_dposv  LAPACKE_dposv 
+
+  // The following functions differ in signature from the 
+  // MXNET_LAPACK-signature and have to be wrapped.
+  #define MXNET_LAPACK_CWRAP_GELQF(prefix, dtype) \
+  inline int MXNET_LAPACK_##prefix##gelqf(int matrix_layout, int m, int n, \
 
 Review comment:
   not dangerous. MKL supports both ways.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] asmushetzel commented on a change in pull request #8577: support for lapack functions with mkl

[GitBox]

asmushetzel commented on a change in pull request #8577: support for lapack 
functions with mkl
URL: https://github.com/apache/incubator-mxnet/pull/8577#discussion_r149603408
 
 

 ##
 File path: make/config.mk
 ##
 @@ -105,6 +105,12 @@ USE_LAPACK = 1
 # path to lapack library in case of a non-standard installation
 USE_LAPACK_PATH =
 
+# by default, disable lapack when using MKL
 
 Review comment:
   yes


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services