Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEAT: Add CSR <--> COO Conversion in all backends #1642

Merged
merged 15 commits into from Dec 8, 2016
Merged

FEAT: Add CSR <--> COO Conversion in all backends #1642

merged 15 commits into from Dec 8, 2016

Conversation

shehzan10
Copy link
Member

@shehzan10 shehzan10 commented Nov 10, 2016

  • Directly convert between AF_STORAGE_COO <--> AF_STORAGE_CSR using the sparseConvertTo function.
    • CPU and OpenCL use native code (no libraries)
    • CUDA uses cusparse
  • Fixed a bug in COO row/col indices from dense
  • Add tests

Fixes CSR <-> COO in #821

[skip arrayfire ci]

@shehzan10 shehzan10 added this to the v3.4.2 milestone Nov 10, 2016
@shehzan10
Copy link
Member Author

build arrayfire windows ci

@@ -307,7 +307,7 @@ af_err af_sparse_convert_to(af_array *out, const af_array in,

// Right now dest_storage can only be AF_STORAGE_DENSE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems out of date. Please update


ovPtr[0] = 0;
for(int x = 0; x < (int)ovalues.elements(); x++) {
int row = std::get<0>(pairKeyVal[x]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a cleaner way to do the same thing:

tie(row, ovPtr[x], ocPtr[x]) = pairKeyVal[x];

@@ -339,18 +339,52 @@ Array<T> sparseConvertStorageToDense(const SparseArray<T> &in_)
////////////////////////////////////////////////////////////////////////////////
// Common to MKL and Not MKL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this comment mean?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 code paths: One that uses MKL and one that is used when compiled without MKL support. This section is common to both as it does not depend on MKL.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess non-MKL would be more meaningful. I understand now.

Array<int> irowIdx = in_.getRowIdx();
Array<int> icolIdx = in_.getColIdx();

kernel::csr_coo<T>()(ovalues, orowIdx, ocolIdx, ivalues, irowIdx, icolIdx);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are making a copy in the function and then you are making a copy during the construction of csr_coo. This seems unnecessary.

@@ -145,6 +145,21 @@ struct nnz_func_def_t
int *, int *);
};

//cusparseStatus_t cusparseZgthr(cusparseHandle_t handle,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in line with other libraries/code we are using where we keep a commented version of the function declaration around as reference.

int *P = memAlloc<int>(nNZ);
CUSPARSE_CHECK(cusparseCreateIdentityPermutation(getHandle(), nNZ, P));

CUSPARSE_CHECK(cusparseXcoosortByColumn(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak on error

converted.getRowIdx().get(), converted.getColIdx().get(),
P, (void*)pBuffer));

CUSPARSE_CHECK(gthr_func<T>()(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memory leak on error

char *pBuffer = memAlloc<char>(pBufferSizeInBytes);

int *P = memAlloc<int>(nNZ);
CUSPARSE_CHECK(cusparseCreateIdentityPermutation(getHandle(), nNZ, P));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak on error.

int *P = memAlloc<int>(nNZ);
CUSPARSE_CHECK(cusparseCreateIdentityPermutation(getHandle(), nNZ, P));

CUSPARSE_CHECK(cusparseXcoosortByRow(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memory leak on error

cooT.getRowIdx().get(), cooT.getColIdx().get(),
P, (void*)pBuffer));

CUSPARSE_CHECK(gthr_func<T>()(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak on error

@shehzan10
Copy link
Member Author

build arrayfire windows ci

@shehzan10 shehzan10 merged commit 6edba84 into arrayfire:hotfix-3.4.2 Dec 8, 2016
@shehzan10 shehzan10 deleted the sparse branch December 8, 2016 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants