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

Tests for powm error #35101

Merged
merged 3 commits into from
Mar 24, 2020
Merged

Tests for powm error #35101

merged 3 commits into from
Mar 24, 2020

Conversation

kshyatt
Copy link
Contributor

@kshyatt kshyatt commented Mar 13, 2020

This little throw and the pass-through for LowerTriangular are not tested.

@kshyatt kshyatt added test This change adds or pertains to unit tests domain:linear algebra Linear algebra labels Mar 13, 2020
@kshyatt kshyatt requested a review from dkarrasch March 13, 2020 16:03
@dkarrasch
Copy link
Member

Wow, this is an amazing example of what testing is good for. The tests currently fail because powm[!] is not exported. BUT, adding a LinearAlgebra specifier still keeps both tests failing, because

  1. ArgumentError("p must be a real number in (-1,1), got $p")
    requires a throw before the ArgumentError, otherwise it simply doesn't throw at all;
  2. powm(A::LowerTriangular, p::Real) = copy(transpose(powm(copy(transpose(A)), p::Real)))
    should be
powm(A::LowerTriangular, p::Real) = copy(transpose(powm!(copy(transpose(A)), p::Real)))

because otherwise it throws saying there's no method matching powm(::UpperTriangular{Complex{Float64},Array{Complex{Float64},2}}, ::Float64). Sure enough, we don't have such a method, only one with !.

@kshyatt Would you be willing to expand this PR to a "bugfix" one, or do you prefer to to do that elsewhere?

@kshyatt
Copy link
Contributor Author

kshyatt commented Mar 13, 2020 via email

@dkarrasch dkarrasch merged commit 9f08f88 into master Mar 24, 2020
@dkarrasch dkarrasch deleted the ksh/powm branch March 24, 2020 13:46
oxinabox pushed a commit to oxinabox/julia that referenced this pull request Apr 8, 2020
ravibitsgoa pushed a commit to ravibitsgoa/julia that referenced this pull request Apr 9, 2020
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: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.

None yet

2 participants