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

[oneMKL][spblas] updated sparse::gemm routines spec, which adds supporting matrix storage layout #363

Merged

Conversation

baeseung-intel
Copy link
Contributor

Goal:
for sparse::gemm() functionality, which is sparse matrix-dense matrix product, support dense matrix storage layout of column-major as well as row-major.

Details:
Currently, there is no parameter related to dense matrix storage layout in spares::gemm() API and assumes the dense matrices are in row-major matrix storage layout. In this PR, I propose to support below features:

  • support layout of dense matrices of matrix B and C for both row-major and column-major.
  • add matrix operation parameter for B matrix, so that we can handle transposed case of matrix B as well as non-transpose case.
  • introduce a new enum class for layout information as oneapi::mkl::layout, which consists of row_major and col_major.

Motivation:
As I mentioned above, current sparse::gemm() API-spec doesn't have parameters for dense matrix storage layout information and and matrix operation on B, which supports only row-major layout and non-transpose operation on matrix B. So if a user needs to call sparse::gemm() with column-major layout dense matrices and/or transpose operation on matrix B, the user has to do extra works before and/or after using sparse::gemm() for their usage.

By updating this proposed sparse::gemm() API, we can support directly for such cases without requiring additional steps. Also, as part of this proposal, I propose another enum class for layout information as well.

…ense matrix storage layout, and introduced layout enum class.
@rscohn2
Copy link
Member

rscohn2 commented Nov 10, 2021

@spencerpatty : Do you want this in 1.1?

@spencerpatty
Copy link
Contributor

yes I think it is probably a good idea to put it in 1.1.

Copy link
Contributor

@spencerpatty spencerpatty left a comment

Choose a reason for hiding this comment

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

I approve of this change.

@spencerpatty spencerpatty merged commit 1744e86 into uxlfoundation:main Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants