-
Notifications
You must be signed in to change notification settings - Fork 6.8k
use mkl sparse matrix to improve performance #14492
Conversation
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.
Great! Thanks for your contribution!
Is there any test case for the sparse matrix?
Could you please provide the comparison on performance?
@mxnet-label-bot add[pr-awaiting-response] |
@mxnet-label-bot add[Sparse, MKL] |
Thanks for your comments. testcase: tests/python/unittest/test_sparse_ndarray.py using mkl , sparse dot will speedup 1.3X With mkl sparse With mxnet sparse Total OP Time: 7883.61600000 ms |
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.
LGTM : ) Thanks for your contribution!
Don't forget to add your name into CONTRIBUTORS.md : )
@pengzhao-intel @TaoLv |
cc @eric-haibin-lin who is the author of the sparse operator. |
@eric-haibin-lin Hi! Would you mind reviewing this PR? Thank you! |
|
||
|
||
|
||
bool mkl_DotCsrDnsDns(SP_INT64* rows_start, SP_INT64* col_indx, |
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.
Why does it land in the 3rdparty directory, instead of src/operator/?
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
- MSHADOW_CFLAGS += -I$(USE_INTEL_PATH)/mkl/include
- MSHADOW_CFLAGS += -I$(USE_INTEL_PATH)/mkl/include -DMKL_ILP64 -m64
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
- MSHADOW_LDFLAGS += -L${MKLROOT}/../compiler/lib/intel64 -Wl,--start-group ${MKLROOT}/lib/intel64/libmkl_intel_lp64.a ${MKLROOT}/lib/intel64/libmkl_core.a ${MKLROOT}/lib/intel64/libmkl_intel_thread.a -Wl,--end-group -liomp5 -ldl -lpthread -lm
- MSHADOW_LDFLAGS += -L${MKLROOT}/../compiler/lib/intel64 -Wl,--start-group ${MKLROOT}/lib/intel64/libmkl_intel_ilp64.a ${MKLROOT}/lib/intel64/libmkl_core.a ${MKLROOT}/lib/intel64/libmkl_intel_thread.a -Wl,--end-group -liomp5 -ldl -lpthread -lm
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
@triplekings could you also share the data shape and nnz distribution of the input you benchmarked with? (whether nonzeros are power-law distributed, or uniformly) |
the model is wide & deep |
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 the windows failure related?
window fails due to unstable CI but not this pr. I have retrigger it again. |
This reverts commit c437d5b.
…pache#14806) This reverts commit c437d5b.
* use mkl sparse matrix to improve performance * fix build fail issue * add 3rdparty/sparse matrix in Makefile * add macro for variable * fix lib not find error * fix gpu R test error * fix Mac build error * add lib/libsparse_matrix.so to CI * fix indentation * retrigger CI
…pache#14806) This reverts commit c437d5b.
Description
(Brief description on what this PR is about)
use mkl sparse matrix replace origin to improve performance
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments