FEAT: sparse-sparse add/sub support #2312
Merged
Conversation
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
|
I have a pending issue on CPU backend that I am currently debugging but the other two backends work as expected. It is probably some typo, will get it soon. Update - Resolved: It was a mistake on my end mixing up custom code with MKL upstream calls that caused the segfaults. |
|
Update: There are some additional changes I need to make to sparse array creation when MKL upstream is used in CPU backend. It involves replacing deprecated API with newest. |
umar456
previously requested changes
Oct 16, 2018
9prady9
force-pushed
the
sparse_arith
branch
2 times, most recently
from
cbf1c41
to
d65af90
Compare
|
I will look into the windows compiler errors tomorrow. |
|
I have fixed windows compilation problems. 🤞 |
9prady9
force-pushed
the
sparse_arith
branch
3 times, most recently
from
83b9101
to
f07b818
Compare
umar456
previously requested changes
Nov 1, 2018
9prady9
force-pushed
the
sparse_arith
branch
from
23fad1f
to
223770e
Compare
9prady9
force-pushed
the
sparse_arith
branch
from
223770e
to
41df2fa
Compare
|
Requires arrayfire/arrayfire-data#12 |
9prady9
force-pushed
the
sparse_arith
branch
from
0c1a3cb
to
c464a69
Compare
9prady9
force-pushed
the
sparse_arith
branch
from
c464a69
to
e93f5b0
Compare
umar456
reviewed
Nov 25, 2018
umar456
previously requested changes
Nov 25, 2018
9prady9
force-pushed
the
sparse_arith
branch
from
e93f5b0
to
bb74531
Compare
9prady9
dismissed
umar456’s stale review
Addressed feedback, review again pls
Latest MKL API, doesn't handle the dense data to sparse storage conversion. It expects pointers to rows, cols and values arrays. It also returns sparse_matrix_t opaque handles that we don't use inside ArrayFire. Hence, deprecated MKL API has been removed in favor of our in-house kernels for conversions.
CPU and OpenCL backends have support for mul/div but they are disabled to have feature parity with CUDA which doesn't have support for mul/div. The output of sub/div/mul is not guaranteed to have only non-zero results of the arithmetic operation. The user has to take care of pruning the zero results from the output.
- Includes tests to read sparse matrix from mtx file cmake configure downloads the compressed mtx files, uncompresses the files and places them under the source tree location `test/data/matrixmarket` so that clean builds don't redownload entire data set. Hence, a new change has been added to test/data git repository to ignore the matrixmarket folder. If for any reason download fails, MTX tests are disabled.
9prady9
force-pushed
the
sparse_arith
branch
from
bb74531
to
3fdd452
Compare
umar456
reviewed
Dec 3, 2018
umar456
approved these changes
Dec 19, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
CPU backend:
CUDA backend sparse-sparse add/sub
OpenCL backend sparse-sparse add/sub/div/mul
The output of sub/div/mul is not guaranteed to have only
non-zero results of the arithmetic operation. The user has
to take care of pruning the zero results from the output.
Support for div/mul isn't exposed to the user yet as it is not uniform across all backends or even within CPU backend at the moment.
Also moved mkl interface defs in CPU backend into single source file