This repository has been archived by the owner on Nov 17, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
use mkl sparse matrix to improve performance #14492
Merged
Merged
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
6d91df0
use mkl sparse matrix to improve performance
triplekings 5628528
fix build fail issue
triplekings 6ca37b4
add 3rdparty/sparse matrix in Makefile
triplekings be07f89
add macro for variable
triplekings 67960a4
fix lib not find error
triplekings d6e5e21
fix gpu R test error
triplekings 7ed707d
fix Mac build error
triplekings eb9c883
add lib/libsparse_matrix.so to CI
triplekings 5fbd8a2
fix indentation
rongzha1 124959f
Merge branch 'master' of https://github.com/apache/incubator-mxnet in…
triplekings f0db599
retrigger CI
triplekings File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
CC = g++ | ||
C = gcc | ||
MKLROOT = /opt/intel/mkl | ||
|
||
ifneq ($(USE_INTEL_PATH),) | ||
MKLROOT = $(USE_INTEL_PATH)/mkl | ||
endif | ||
|
||
CFLAGS = -fpic -O2 -I/opt/intel/mkl/include -c -Wall -Werror -DMKL_ILP64 -m64 -std=c++11 | ||
LDFLAGS = -Wl,--start-group -L${MKLROOT}/../compiler/lib/intel64 ${MKLROOT}/lib/intel64/libmkl_intel_ilp64.a ${MKLROOT}/lib/intel64/libmkl_intel_thread.a ${MKLROOT}/lib/intel64/libmkl_core.a -Wl,--end-group -liomp5 -lpthread -lm -ldl | ||
|
||
default: libsparse_matrix.so | ||
|
||
libsparse_matrix.so: sparse_matrix.o | ||
$(CC) -shared -o libsparse_matrix.so sparse_matrix.o $(LDFLAGS) | ||
|
||
sparse_matrix.o: sparse_matrix.cc sparse_matrix.h | ||
$(CC) $(CFLAGS) sparse_matrix.cc | ||
|
||
clean: | ||
$(RM) libsparse_matrix.so *.o *~ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
#include <iostream> | ||
#include <string> | ||
#include <fstream> | ||
#include <mkl_spblas.h> | ||
#include "sparse_matrix.h" | ||
|
||
|
||
|
||
bool mkl_DotCsrDnsDns(SP_INT64* rows_start, SP_INT64* col_indx, | ||
float* values, float* X, float* y, | ||
int rows, int cols, int X_columns) | ||
{ | ||
|
||
sparse_index_base_t indexing = SPARSE_INDEX_BASE_ZERO; | ||
sparse_status_t status; | ||
sparse_matrix_t A = NULL; | ||
sparse_layout_t layout = SPARSE_LAYOUT_ROW_MAJOR; | ||
float one, zero; | ||
eric-haibin-lin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
one = (float)1.0; | ||
zero = (float)0.0; | ||
|
||
MKL_INT* rows_end = rows_start + 1; | ||
status = mkl_sparse_s_create_csr(&A, indexing, rows, cols, rows_start, rows_end, col_indx, values); | ||
|
||
if (status != SPARSE_STATUS_SUCCESS) | ||
{ | ||
std::cout << "mkl_sparse_s_create_csr status :" << status << std::endl; | ||
return false; | ||
} | ||
sparse_operation_t operation = SPARSE_OPERATION_NON_TRANSPOSE; | ||
struct matrix_descr descrA; | ||
descrA.type = SPARSE_MATRIX_TYPE_GENERAL; | ||
|
||
status = mkl_sparse_s_mm(operation, one, A, descrA, layout, X, X_columns, X_columns, zero, y, X_columns); | ||
if (status != SPARSE_STATUS_SUCCESS) | ||
{ | ||
std::cout << "mkl_sparse_s_create_csr status :" << status << std::endl; | ||
eric-haibin-lin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return false; | ||
} | ||
|
||
mkl_sparse_destroy(A); | ||
|
||
return true; | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
#ifndef MXNET_OPERATOR_SPARSE_MATRIX_INL_H_ | ||
#define MXNET_OPERATOR_SPARSE_MATRIX_INL_H_ | ||
|
||
|
||
#if (!defined(__INTEL_COMPILER)) & defined(_MSC_VER) | ||
#define SP_INT64 __int64 | ||
#define SP_UINT64 unsigned __int64 | ||
#else | ||
#define SP_INT64 long long int | ||
#define SP_UINT64 unsigned long long int | ||
#endif | ||
|
||
|
||
#if defined _WIN32 || defined __CYGWIN__ | ||
#ifdef BUILDING_DLL | ||
#ifdef __GNUC__ | ||
#define SPM_API_PUBLIC __attribute__ ((dllexport)) | ||
#else | ||
#define SPM_API_PUBLIC __declspec(dllexport) // Note: actually gcc seems to also supports this syntax. | ||
#endif | ||
#else | ||
#ifdef __GNUC__ | ||
#define SPM_API_PUBLIC __attribute__ ((dllimport)) | ||
#else | ||
#define SPM_API_PUBLIC __declspec(dllimport) // Note: actually gcc seems to also supports this syntax. | ||
#endif | ||
#endif | ||
#define SPM_API_LOCAL | ||
#else | ||
#if __GNUC__ >= 4 | ||
#define SPM_API_PUBLIC __attribute__ ((visibility ("default"))) | ||
#define SPM_API_LOCAL __attribute__ ((visibility ("hidden"))) | ||
#else | ||
#define SPM_API_PUBLIC | ||
#define SPM_API_LOCAL | ||
#endif | ||
#endif | ||
|
||
|
||
|
||
extern "C" | ||
{ | ||
extern SPM_API_PUBLIC bool mkl_DotCsrDnsDns(SP_INT64* rows_start, SP_INT64* col_indx, | ||
float* values, float* X, float* y, int rows, int cols, int X_columns); | ||
|
||
} | ||
|
||
#endif //MXNET_OPERATOR_SPARSE_MATRIX_INL_H_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does it land in the 3rdparty directory, instead of src/operator/?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there future plan to support other sparse matrix ops, like dense + csr, csr + csr, csr * csr (elemwise)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eric-haibin-lin
If you requests, e.g. mkl has
mkl_sparse_spmm s, d, c, z Computes the product of two sparse matrices and stores
the result as a sparse matrix
why this patch is merge first is because Wide & Deep use sparse dot dense matrix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the quick solution for the wide deep. To move the code into src/operator, need to change code in mshadow and more tests are needed. I will do it as the next step.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation. Why mshadow has to be changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mshadw use 32bit interface , below is the link for mkl settings
https://software.intel.com/en-us/articles/intel-mkl-link-line-advisor
and the make should change
diff --git a/make/mshadow.mk b/make/mshadow.mk
index 86155ea..d0b731e 100644
--- a/make/mshadow.mk
+++ b/make/mshadow.mk
@@ -82,7 +82,7 @@ ifneq ($(USE_INTEL_PATH), NONE)
MSHADOW_LDFLAGS += -L$(USE_INTEL_PATH)/compiler/lib/intel64
MSHADOW_LDFLAGS += -L$(USE_INTEL_PATH)/lib/intel64
endif
endif
ifneq ($(USE_STATIC_MKL), NONE)
ifeq ($(USE_INTEL_PATH), NONE)
@@ -90,7 +90,7 @@ ifeq ($(USE_INTEL_PATH), NONE)
else
MKLROOT = $(USE_INTEL_PATH)/mkl
endif
else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look forward to the followup PR