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

Fill out linalg tests #12095

Merged
merged 4 commits into from
Jul 10, 2015
Merged

Fill out linalg tests #12095

merged 4 commits into from
Jul 10, 2015

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented Jul 10, 2015

I found a bug in taking the inverse of an Eigen which is hopefully fixed now. I also added tests to get some of base/linalg to 100% coverage.

@kshyatt kshyatt added test This change adds or pertains to unit tests linear algebra Linear algebra labels Jul 10, 2015
@andreasnoack
Copy link
Member

A long lived one. I made that error in March 2013. We should (again) consider saving the symmetric and general eigensolutions as separate types. It is a shame to invert the vectors when you know that the problem is symmetric, but that is for another pr. Thanks for fixing this.

@kshyatt
Copy link
Contributor Author

kshyatt commented Jul 10, 2015

Looks like I got hit by the AppVeyor timeout. Everyone ok with this change?

@ViralBShah
Copy link
Member

Nice find. I think given its passing on travis, should be good to merge.

@kshyatt
Copy link
Contributor Author

kshyatt commented Jul 10, 2015

But, but, #12086 (comment).

Also I think this PR really demonstrates the value of getting files to 100% coverage, considering base/linalg/eigen.jl had only 5 lines untested and yet this little 🐛 was hiding in there.

@StefanKarpinski
Copy link
Member

Figuring out how to get to 100% coverage is definitely something we should do.

@tkelman
Copy link
Contributor

tkelman commented Jul 10, 2015

restarted, passing now

agreed with @andreasnoack that symmetric/hermitian eigenproblems should probably be a different type than nonsymmetric some day

kshyatt added a commit that referenced this pull request Jul 10, 2015
@kshyatt kshyatt merged commit 14af2d1 into master Jul 10, 2015
@kshyatt kshyatt deleted the ksh/linalgtests branch July 10, 2015 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants