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

RFC: add complex-valued tests #2845

Merged
merged 7 commits into from
Apr 17, 2013
Merged

Conversation

simonbyrne
Copy link
Contributor

I've started adding some tests for complex valued functions, to deal with branch cut issues (see #2843).

Currently just has tests for sqrt. All tests are taken from the mac csqrt man page (it was the most specific).

Some of these tests currently fail (which I guess is the point of doing them). I'm happy to add more functions (log, trig/inverse trig, etc.).

@ViralBShah
Copy link
Member

Thanks. Please add as many as you can. Let's fix all the uncovered issues in here and then merge.

@simonbyrne
Copy link
Contributor Author

Okay, there's a few more. I'll add more as I get time.

@simonbyrne
Copy link
Contributor Author

I've included the ones which are supposed to "raise an invalid flag": some of these currently raise an error, e.g.

julia> sinh(complex(0.0,Inf))
ERROR: DomainError()
 in sinh at complex.jl:332

Is there a standard method for asserting an error in the tests?

@aviks
Copy link
Member

aviks commented Apr 12, 2013

julia> using Base.Test

julia> @test_fails sinh(complex(0.0,Inf))

julia>

@ViralBShah
Copy link
Member

Ideally we need a way to check that the particular exception was raised, rather than a generic error.

@ViralBShah
Copy link
Member

Here, we really need the feature that was requested earlier (was it @johnmyleswhite ?) that the tests should continue running even if certain tests fail, so that we can see all the failures. This would be especially useful in travis.

@andrioni
Copy link
Member

This could be solved by using a more appropriate handler when needed, in fact this was exactly my reason for #2680.

@ViralBShah
Copy link
Member

@andrioni Good point.

@simonbyrne
Copy link
Contributor Author

Okay, that should be all of the functions. The only one which is incomplete is ^: this is a bit tricky, as it's a binary function, and not really clearly defined in the standard.

To make this a bit easier to incorporate, I could remove it from runtests.jl so it won't be called by travis? It could still be called manually by make test-complex.

@ViralBShah
Copy link
Member

We can't merge this before all of these pass. Do you have time to look at and fix the implementations too?

Also cc: @jiahao to help out.

@ViralBShah
Copy link
Member

An alternate thing to do would be to put all these new tests in a new file. We certainly do not want to disable existing tests. Then we can keep fixing them and moving them to test/complex.jl.

@ViralBShah
Copy link
Member

This also needs a rebase before merging, once the new tests are put in say, complex-bc.jl.

@simonbyrne
Copy link
Contributor Author

complex.jl is a new file, so disabling it won't disable any existing tests.

I'm happy to help out with the implementations where I can, but some of them may be beyond my skill set.

@simonbyrne
Copy link
Contributor Author

Okay, I've rebased, and removed the new tests from make testall (but they can still be run by make test-complex).

ViralBShah pushed a commit that referenced this pull request Apr 17, 2013
RFC: add complex-valued tests
@ViralBShah ViralBShah merged commit a91476d into JuliaLang:master Apr 17, 2013
@simonbyrne simonbyrne deleted the complextest branch April 17, 2013 08:08
@simonbyrne
Copy link
Contributor Author

To keep the consistency of the limits:

julia> complex(1e-10,1e-10)^complex(1e-10,1e-10)
0.9999999976535324 - 2.189387912488976e-9im

@simonbyrne
Copy link
Contributor Author

But as I said, ^ isn't really specified that clearly: the mac documentations for cpow says that cpow(x,y) == cexp(y * clog(x)). However for x=0 + i0, clog(x) == -Inf + 0i, which means that the term inside the y*clog(x) would have an NaN.

The C99 standard isn't really specific, just saying (in a footnote):

This allows cpow(z, c) to be implemented as cexp(c clog(z)) without precluding
implementations that treat special cases more carefully.

# raise invalid flag
@test_fails exp(complex(0.0,Inf) #complex(NaN,NaN)
@test_fails exp(complex(0.0,-Inf) #complex(NaN,NaN)
@test_fails exp(complex(5.0,Inf) #complex(NaN,NaN)
Copy link
Member

Choose a reason for hiding this comment

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

These lines lack closing parentheses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, these were fixed by #2878.

@andrioni andrioni mentioned this pull request Apr 26, 2013
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants