Skip to content

Conversation

@dkarrasch
Copy link
Member

@dkarrasch dkarrasch commented Jun 24, 2019

Closes #32402.

A quick adoption of the existing Transpose method, plus some minor code simplification/consistency.

@dkarrasch dkarrasch changed the title WIP: add specific Adjoint{StridedMatrix} * SparseVector method add specific Adjoint{StridedMatrix} * SparseVector method Jun 25, 2019
@dkarrasch
Copy link
Member Author

Test failure on windows is in download.

@ViralBShah ViralBShah added sparse Sparse arrays performance Must go faster labels Jun 25, 2019
@ViralBShah
Copy link
Member

Do we have adequate test coverage of these cases?

@dkarrasch
Copy link
Member Author

Do we have adequate test coverage of these cases?

What do you mean? We had tests for denseMat*sparseVec and transposeDenseMat * sparseVec. I added tests to cover Adjoints and added Complex eltypes where Transpose and Adjoint make a difference.

@ViralBShah
Copy link
Member

Looks good to me.

@dkarrasch
Copy link
Member Author

If anything, then I see the "issue" of code duplication. I see that we usually don't make much effort in code reduction in the area of matrix multiplication, because of the risk of method ambiguity. But I guess if code reduction is desired this could be another PR.

Just to give an example, the 3-arg mul! simply forwards its arguments to the 5-arg mul! by appending one and zero. Currently, we have 3 individual methods for that, for StridedMatrix, Adjoint and Transpose. Other reductions include the trick we sometimes use in tests, for-loop over transform in (adjoint, transpose) and then @eval and the function definition etc.

@ViralBShah
Copy link
Member

I would be happy to do code reduction separately and get performance improvements in for now.

@ViralBShah ViralBShah merged commit f7f482b into JuliaLang:master Jun 26, 2019
@dkarrasch dkarrasch deleted the dk/adjmulsparsevec branch June 26, 2019 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Must go faster sparse Sparse arrays

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slow matrix vector product (adjoint dense * SparseCSC)

2 participants