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

Linear algebra test coverage for SubArray #299

Closed
15 of 22 tasks
andreasnoack opened this issue Jan 19, 2016 · 17 comments
Closed
15 of 22 tasks

Linear algebra test coverage for SubArray #299

andreasnoack opened this issue Jan 19, 2016 · 17 comments
Labels
good first issue Good for newcomers Hacktoberfest Good for Hacktoberfest participants test This change adds or pertains to unit tests

Comments

@andreasnoack
Copy link
Member

andreasnoack commented Jan 19, 2016

Below is the list of linear algebra test files that we'll need to extend with SubArray tests. Please update the list if SubArrays are already covered (systematically) for the file

@kshyatt kshyatt added test This change adds or pertains to unit tests linear algebra good first issue Good for newcomers labels Jan 19, 2016
@andreasnoack
Copy link
Member Author

@kshyatt Are tridiagonal, bidiagonal and diagonal now covered? If so, please update the list.

@pkofod
Copy link

pkofod commented Feb 18, 2016

just to be 100% sure, this is simply to make sure that the functions return the same thing / work even if it's a SubArray and not an array?

@kshyatt
Copy link
Contributor

kshyatt commented Feb 18, 2016

Yeah, the idea is to make sure Diagonal {*, +, -, /} SubArray still works.

@pkofod
Copy link

pkofod commented Mar 3, 2016

svd.jl should be covered by JuliaLang/julia#15314 .

@andreasnoack
Copy link
Member Author

Great. Top post is now updated.

@pkofod
Copy link

pkofod commented Mar 3, 2016

I'll probably grab some more the coming days, and pile them in a PR with separate commits. I assume none are taken?

@tkelman
Copy link

tkelman commented Mar 4, 2016

Nope, go nuts. Keep em coming @pkofod, big fan of your recent contributions.

@pkofod
Copy link

pkofod commented Mar 4, 2016

Most of arnoldi.jl is sparse matrices. Should these just be ignored here? Methods don't seem to be defined for sparse matrices (#15355 ) possibly because it will rarely be a good idea to operate on views of sparse matrices.

@tkelman
Copy link

tkelman commented Mar 5, 2016

I don't think this is intentional, it's just that few if any methods for working with views of sparse matrices have been written yet.

@pkofod
Copy link

pkofod commented Mar 5, 2016

As pointed out by Tim Holy at JuliaLang/julia#15354, it works in JuliaLang/julia#15367

@tkelman
Copy link

tkelman commented Mar 6, 2016

Though if it's dispatching to a generic fallback, it's likely quite slow and not taking advantage of sparsity, so optimized methods do still need to be written to get decent performance.

@pkofod
Copy link

pkofod commented Mar 6, 2016

Yes I realize that, I was only thinking about: "does it work?". If it is expected to work, I figured it should be tested. Even if it would probably be stupid for most users to do it currently.

@pkofod
Copy link

pkofod commented May 25, 2016

Regarding this issue... Is it "acceptable" to continue down this road right now, or will a PR that increases test times not be merged anyway? (I'm thinking about the whole CI discussion...) Also, I think there are some completed files that are not ticked off (matmul, eigen, schur, bunchkaufman).

@andreasnoack
Copy link
Member Author

I don't think we should hold back on adding tests here but @tkelman can tell if we are too close to the limit. Recently, the triangular tests got faster after avoiding many calls to generic_matmatmul! so maybe there is some extra room. It might be possible to further reduce the number of times generic_matmatmul! is compiled or reorganize that function to become cheaper to compile.

@tkelman
Copy link

tkelman commented May 25, 2016

The time per CI run has gotten about 20 minutes faster over the last few months, which is great work by the compiler gurus. But we are having more frequent freezes and it's generally busy with a lot of builds per day adding up to a usually-long queue.

@kshyatt
Copy link
Contributor

kshyatt commented Jun 8, 2016

I think lq.jl is actually covered? Since I pulled those tests from qr.jl it may be as well. See here.

@ViralBShah
Copy link
Member

ViralBShah commented Oct 31, 2020

@kshyatt Are we now in a good enough place, that we can close this?

@KristofferC KristofferC transferred this issue from JuliaLang/julia Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Hacktoberfest Good for Hacktoberfest participants test This change adds or pertains to unit tests
Projects
None yet
Development

No branches or pull requests

5 participants