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

added sparse multiplication and division for triangular matrices. Fix… #28507

Merged
merged 14 commits into from
Oct 8, 2018
Merged

added sparse multiplication and division for triangular matrices. Fix… #28507

merged 14 commits into from
Oct 8, 2018

Conversation

KlausC
Copy link
Contributor

@KlausC KlausC commented Aug 7, 2018

The performance from #28451 has been improved for some types of sparse triangular matrices.
All the new performance figures are now in range 1-2ms, means improvements by factors 300-1000. See #28451.

@kshyatt kshyatt added linear algebra Linear algebra sparse Sparse arrays labels Aug 13, 2018
@chriscoey
Copy link

thank you @KlausC

impatient n00b here, can this be merged?

@KlausC
Copy link
Contributor Author

KlausC commented Aug 27, 2018

@mcognetta , @andreasnoack may I bring the PR back to your mind.
It provides improved algorithms for multiplication and division of Triangular and Unit-Triangular sparse matrices with extreme performance factors.
I did not see all of these cases covered by #28883

@mcognetta
Copy link
Contributor

Thanks, this is really nice. I will merge this into #28883.

mcognetta added a commit to mcognetta/julia that referenced this pull request Aug 30, 2018
…ns' into optimize_binop_structured_matrices

Adding sparse triangular sparse array operations from JuliaLang#28507
@KlausC
Copy link
Contributor Author

KlausC commented Sep 10, 2018

@andreasnoack is there a reason, why this is not merged into master?

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Sep 10, 2018

I suspect because it's a fair amount of code to review. @andreasnoack has got a few other things I happen to know he's working on somewhat urgently at the moment. Have patience, we'll have more cycles available next week. @Sacha0, any chance you could take a look?

@chriscoey
Copy link

@andreasnoack @Sacha0 it's a little discouraging that important linear algebra improvements like this get forgotten

@Sacha0
Copy link
Member

Sacha0 commented Oct 4, 2018

Regrettably I have few spare cycles to review pull requests of this scale at the moment. Best!

Copy link
Member

@andreasnoack andreasnoack left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks for the contribution and sorry for the long wait here. I've only a minor comment to the tests.

A = sprand(rng, n, n, 0.01)
B = ones(n)
MA = Matrix(A)
for tr in (identity, adjoint, transpose)
Copy link
Member

Choose a reason for hiding this comment

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

It would be great if you could put a testset here that prints the loop variables. E.g. just something like

@testset "tr=$tr, wr=$wr" for
    tr in (identity, adjoint, transpose),
        wr in (UpperTriangular, LowerTriangular, UnitUpperTriangular, UnitLowerTriangular)

helps a lot to identify the problem when a test fails.

@andreasnoack andreasnoack merged commit 361c5e7 into JuliaLang:master Oct 8, 2018
@KlausC KlausC deleted the sparsearrays-triangular-operations branch October 8, 2018 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra sparse Sparse arrays
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants