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

Fix log1p with complex Irrational argument #21784

Merged
merged 1 commit into from
May 14, 2017

Conversation

giordano
Copy link
Contributor

Even if you don't like Complex{Irrational}, this change is a slight performance improvement, because after PR #21219 float(T) doesn't allocate with bignumbers and Rationals

@musm
Copy link
Contributor

musm commented May 10, 2017

Unrelated but isn't it strange that float has multiple meanings whether it acts on Types or Data?
float(2) = 2.0
float(Int) = Float64
Is this a common thing for methods to have multiple meanings depending on whether the arguments are Types or Data values

@giordano
Copy link
Contributor Author

giordano commented May 10, 2017

It looks like not everybody like this (see, e.g., #21218 (comment)), but this is useful to define more efficient methods, like in this case. In addition, you often want to determine the floating type corresponding to T, the alternative is to use the fallback definition of float, that is typeof(float(zero(T))), but it may not be possible to define zero(T) for all T (like Irrational).

@giordano giordano mentioned this pull request May 10, 2017
5 tasks
@ararslan ararslan added the maths Mathematical functions label May 10, 2017
Copy link
Contributor

@tkelman tkelman left a comment

Choose a reason for hiding this comment

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

lgtm

@tkelman tkelman merged commit 320892a into JuliaLang:master May 14, 2017
@giordano giordano deleted the log1p-irrational branch May 14, 2017 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maths Mathematical functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants