You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@systemds.apache.org by GitBox <gi...@apache.org> on 2020/11/14 10:11:44 UTC
[GitHub] [systemds] Baunsgaard commented on a change in pull request #1101: [SYSTEMDS-2725] NNZ counting for native blas
Baunsgaard commented on a change in pull request #1101:
URL: https://github.com/apache/systemds/pull/1101#discussion_r523401825
##########
File path: src/main/cpp/libmatrixmult.cpp
##########
@@ -18,70 +18,42 @@
*/
#include "libmatrixmult.h"
-#include "omp.h"
-#include <cmath>
-#include <cstdlib>
-#ifdef USE_OPEN_BLAS
-#include <cblas.h>
-#else
-#include <mkl_service.h>
-#endif
+#include "common.h"
-static int SYSDS_CURRENT_NUM_THREADS = -1;
-void setNumThreadsForBLAS(int numThreads) {
- if (SYSDS_CURRENT_NUM_THREADS != numThreads) {
-#ifdef USE_OPEN_BLAS
- openblas_set_num_threads(numThreads);
-#else
- mkl_set_num_threads(numThreads);
-#endif
- SYSDS_CURRENT_NUM_THREADS = numThreads;
- }
-}
-
-void dmatmult(double *m1Ptr, double *m2Ptr, double *retPtr, int m, int k, int n,
- int numThreads) {
+void dmatmult(double *m1Ptr, double *m2Ptr, double *retPtr, int m, int k, int n, int numThreads) {
// BLAS routine dispatch according to input dimension sizes (we don't use
// cblas_dgemv with CblasColMajor for matrix-vector because it was generally
// slower than dgemm)
setNumThreadsForBLAS(numThreads);
if (m == 1 && n == 1) // VV
- retPtr[0] = cblas_ddot(k, m1Ptr, 1, m2Ptr, 1);
+ retPtr[0] = cblas_ddot(k, m1Ptr, 1, m2Ptr, 1);
Review comment:
Something went wrong with indentation in this file.
##########
File path: src/main/cpp/libmatrixmult.h
##########
@@ -17,48 +17,19 @@
* under the License.
*/
-#ifndef _libmatrixmult_h
-#define _libmatrixmult_h
+#ifndef LIBMATRIXMULT_H
+#define LIBMATRIXMULT_H
Review comment:
is there a convention for this, all CAPS?
##########
File path: src/main/cpp/systemds.cpp
##########
@@ -170,17 +158,17 @@ JNIEXPORT jint JNICALL Java_org_apache_sysds_utils_NativeHelper_conv2dDense(
if(inputPtr == NULL || filterPtr == NULL || retPtr == NULL)
return (jint) -1;
- int nnz = dconv2dBiasAddDense(inputPtr, 0, filterPtr, retPtr,
- (int) N, (int) C, (int) H, (int) W, (int) K, (int) R, (int) S,
- (int) stride_h, (int) stride_w, (int) pad_h, (int) pad_w, (int) P, (int) Q, false, (int) numThreads);
+ size_t nnz = dconv2dBiasAddDense(inputPtr, 0, filterPtr, retPtr, (int) N, (int) C, (int) H, (int) W, (int) K,
+ (int) R, (int) S, (int) stride_h, (int) stride_w, (int) pad_h, (int) pad_w, (int) P,
+ (int) Q, false, (int) numThreads);
Review comment:
Indentation broken
##########
File path: src/main/cpp/systemds.h
##########
@@ -29,65 +29,65 @@ extern "C" {
/*
* Class: org_apache_sysds_utils_NativeHelper
* Method: dmmdd
- * Signature: ([D[D[DIIII)Z
+ * Signature: ([D[D[DIIII)I
Review comment:
this docs seems wierd.
does the coding of this make a difference for the docs constructed?
otherwise i would consider using double* and so on.
##########
File path: src/main/cpp/systemds.cpp
##########
@@ -68,57 +51,62 @@
// -------------------------------------------------------------------
-int maxThreads = -1;
+//int maxThreads = -1;
JNIEXPORT void JNICALL Java_org_apache_sysds_utils_NativeHelper_setMaxNumThreads
(JNIEnv *, jclass, jint jmaxThreads) {
- maxThreads = (int) jmaxThreads;
+// maxThreads = (int) jmaxThreads;
+ setNumThreadsForBLAS(jmaxThreads);
}
-JNIEXPORT jboolean JNICALL Java_org_apache_sysds_utils_NativeHelper_dmmdd(
+JNIEXPORT jlong JNICALL Java_org_apache_sysds_utils_NativeHelper_dmmdd(
JNIEnv* env, jclass cls, jdoubleArray m1, jdoubleArray m2, jdoubleArray ret,
jint m1rlen, jint m1clen, jint m2clen, jint numThreads)
{
double* m1Ptr = GET_DOUBLE_ARRAY(env, m1, numThreads);
double* m2Ptr = GET_DOUBLE_ARRAY(env, m2, numThreads);
double* retPtr = GET_DOUBLE_ARRAY(env, ret, numThreads);
if(m1Ptr == NULL || m2Ptr == NULL || retPtr == NULL)
- return (jboolean) false;
+ return -1;
Review comment:
don't you need to cast to jlong here as well
##########
File path: src/main/cpp/systemds.cpp
##########
@@ -68,57 +51,62 @@
// -------------------------------------------------------------------
-int maxThreads = -1;
+//int maxThreads = -1;
JNIEXPORT void JNICALL Java_org_apache_sysds_utils_NativeHelper_setMaxNumThreads
(JNIEnv *, jclass, jint jmaxThreads) {
- maxThreads = (int) jmaxThreads;
+// maxThreads = (int) jmaxThreads;
+ setNumThreadsForBLAS(jmaxThreads);
}
-JNIEXPORT jboolean JNICALL Java_org_apache_sysds_utils_NativeHelper_dmmdd(
+JNIEXPORT jlong JNICALL Java_org_apache_sysds_utils_NativeHelper_dmmdd(
JNIEnv* env, jclass cls, jdoubleArray m1, jdoubleArray m2, jdoubleArray ret,
jint m1rlen, jint m1clen, jint m2clen, jint numThreads)
{
double* m1Ptr = GET_DOUBLE_ARRAY(env, m1, numThreads);
double* m2Ptr = GET_DOUBLE_ARRAY(env, m2, numThreads);
double* retPtr = GET_DOUBLE_ARRAY(env, ret, numThreads);
if(m1Ptr == NULL || m2Ptr == NULL || retPtr == NULL)
- return (jboolean) false;
+ return -1;
dmatmult(m1Ptr, m2Ptr, retPtr, (int)m1rlen, (int)m1clen, (int)m2clen, (int)numThreads);
+ size_t nnz = computeNNZ<double>(retPtr, m1rlen * m2clen);
RELEASE_INPUT_ARRAY(env, m1, m1Ptr, numThreads);
RELEASE_INPUT_ARRAY(env, m2, m2Ptr, numThreads);
RELEASE_ARRAY(env, ret, retPtr, numThreads);
- return (jboolean) true;
+
+ return static_cast<jlong>(nnz);
}
-JNIEXPORT jboolean JNICALL Java_org_apache_sysds_utils_NativeHelper_smmdd(
+JNIEXPORT jlong JNICALL Java_org_apache_sysds_utils_NativeHelper_smmdd(
JNIEnv* env, jclass cls, jobject m1, jobject m2, jobject ret,
jint m1rlen, jint m1clen, jint m2clen, jint numThreads)
{
float* m1Ptr = (float*) env->GetDirectBufferAddress(m1);
float* m2Ptr = (float*) env->GetDirectBufferAddress(m2);
float* retPtr = (float*) env->GetDirectBufferAddress(ret);
if(m1Ptr == NULL || m2Ptr == NULL || retPtr == NULL)
- return (jboolean) false;
+ return -1;
smatmult(m1Ptr, m2Ptr, retPtr, (int)m1rlen, (int)m1clen, (int)m2clen, (int)numThreads);
- return (jboolean) true;
+ return static_cast<jlong>(computeNNZ<float>(retPtr, m1rlen * m2clen));
}
-JNIEXPORT jboolean JNICALL Java_org_apache_sysds_utils_NativeHelper_tsmm
+JNIEXPORT jlong JNICALL Java_org_apache_sysds_utils_NativeHelper_tsmm
(JNIEnv * env, jclass cls, jdoubleArray m1, jdoubleArray ret, jint m1rlen, jint m1clen, jboolean leftTrans, jint numThreads) {
double* m1Ptr = GET_DOUBLE_ARRAY(env, m1, numThreads);
double* retPtr = GET_DOUBLE_ARRAY(env, ret, numThreads);
if(m1Ptr == NULL || retPtr == NULL)
- return (jboolean) false;
+ return -1;
Review comment:
cast here as well
##########
File path: src/main/cpp/systemds.cpp
##########
@@ -68,57 +51,62 @@
// -------------------------------------------------------------------
-int maxThreads = -1;
+//int maxThreads = -1;
JNIEXPORT void JNICALL Java_org_apache_sysds_utils_NativeHelper_setMaxNumThreads
(JNIEnv *, jclass, jint jmaxThreads) {
- maxThreads = (int) jmaxThreads;
+// maxThreads = (int) jmaxThreads;
Review comment:
i would remove these commented lines
##########
File path: src/main/cpp/systemds.cpp
##########
@@ -192,18 +180,18 @@ JNIEXPORT jint JNICALL Java_org_apache_sysds_utils_NativeHelper_dconv2dBiasAddDe
if(inputPtr == NULL || biasPtr == NULL || filterPtr == NULL || retPtr == NULL)
return (jint) -1;
- int nnz = dconv2dBiasAddDense(inputPtr, biasPtr, filterPtr, retPtr,
- (int) N, (int) C, (int) H, (int) W, (int) K, (int) R, (int) S,
- (int) stride_h, (int) stride_w, (int) pad_h, (int) pad_w, (int) P, (int) Q, true, (int) numThreads);
+ size_t nnz = dconv2dBiasAddDense(inputPtr, biasPtr, filterPtr, retPtr, (int) N, (int) C, (int) H, (int) W, (int) K,
+ (int) R, (int) S, (int) stride_h, (int) stride_w, (int) pad_h, (int) pad_w, (int) P,
+ (int) Q, true, (int) numThreads);
Review comment:
indent
##########
File path: src/main/cpp/systemds.cpp
##########
@@ -215,14 +203,14 @@ JNIEXPORT jint JNICALL Java_org_apache_sysds_utils_NativeHelper_sconv2dBiasAddDe
if(inputPtr == NULL || biasPtr == NULL || filterPtr == NULL || retPtr == NULL)
return (jint) -1;
- int nnz = sconv2dBiasAddDense(inputPtr, biasPtr, filterPtr, retPtr,
- (int) N, (int) C, (int) H, (int) W, (int) K, (int) R, (int) S,
- (int) stride_h, (int) stride_w, (int) pad_h, (int) pad_w, (int) P, (int) Q, true, (int) numThreads);
-
- return (jint) nnz;
+ size_t nnz = sconv2dBiasAddDense(inputPtr, biasPtr, filterPtr, retPtr, (int) N, (int) C, (int) H, (int) W, (int) K,
+ (int) R, (int) S, (int) stride_h, (int) stride_w, (int) pad_h, (int) pad_w, (int) P,
+ (int) Q, true, (int) numThreads);
+
+ return static_cast<jlong>(nnz);
}
-JNIEXPORT jint JNICALL Java_org_apache_sysds_utils_NativeHelper_conv2dBackwardDataDense(
+JNIEXPORT jlong JNICALL Java_org_apache_sysds_utils_NativeHelper_conv2dBackwardDataDense(
JNIEnv* env, jclass, jdoubleArray filter, jdoubleArray dout,
jdoubleArray ret, jint N, jint C, jint H, jint W, jint K, jint R, jint S,
jint stride_h, jint stride_w, jint pad_h, jint pad_w, jint P, jint Q, jint numThreads) {
Review comment:
Indent
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org