Skip to content

Use iszero and isone in MPFR #23322

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

Merged
merged 2 commits into from
Aug 25, 2017
Merged

Use iszero and isone in MPFR #23322

merged 2 commits into from
Aug 25, 2017

Conversation

musm
Copy link
Contributor

@musm musm commented Aug 18, 2017

No allocations and is faster

julia> x = BigFloat(0)

julia> @btime iszero($x)
  9.474 ns (0 allocations: 0 bytes)
true

julia> f(x) = x == 0
f (generic function with 1 method)

julia> @btime f($x)
  99.874 ns (2 allocations: 56 bytes)
true

@musm musm changed the title Use iszero and is one in mpfr Use iszero and isone in MPFR Aug 18, 2017
@@ -275,7 +275,7 @@ big(::Type{<:AbstractFloat}) = BigFloat
function convert(::Type{Rational{BigInt}}, x::AbstractFloat)
if isnan(x); return zero(BigInt)//zero(BigInt); end
if isinf(x); return copysign(one(BigInt),x)//zero(BigInt); end
if x == 0; return zero(BigInt) // one(BigInt); end
if iszero(x); return zero(BigInt) // one(BigInt); end
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps nix the extra spaces and/or restore alignment with the preceding lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have another PR that changes this, which will conflict anyways. So I prefer to see one merge first before making any additional changes.

Copy link
Member

@Sacha0 Sacha0 left a comment

Choose a reason for hiding this comment

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

lgtm modulo the minor comments! :)

@giordano
Copy link
Member

I don't have anything against this patch (actually, I'm in favor of it), but on Julia 0.6 I see a fairly different benchmark result:

julia> x = BigFloat(0)
0.000000000000000000000000000000000000000000000000000000000000000000000000000000

julia> @btime iszero($x)
  9.517 ns (0 allocations: 0 bytes)
true

julia> f(x) = x == 0
f (generic function with 1 method)

julia> @btime f($x)
  9.763 ns (0 allocations: 0 bytes)
true

(is BenchmarkTools still broken on master?)

@musm
Copy link
Contributor Author

musm commented Aug 20, 2017

@giordano My benchmarks used the commit right before BenchmarkTools broke on master

@giordano
Copy link
Member

giordano commented Aug 20, 2017

On 952dc93, on my machine:

julia> x = BigFloat(0)
0.000000000000000000000000000000000000000000000000000000000000000000000000000000

julia> @btime iszero($x)
  9.453 ns (0 allocations: 0 bytes)
true

julia> f(x) = x == 0
f (generic function with 1 method)

julia> @btime f($x)
  9.167 ns (0 allocations: 0 bytes)
true

@musm
Copy link
Contributor Author

musm commented Aug 24, 2017

maybe we should run the BigFloat benchmarks here

@andreasnoack
Copy link
Member

@nanosoldier runbenchmarks("BigFloat", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @ararslan

@rfourquet rfourquet merged commit c44461a into JuliaLang:master Aug 25, 2017
@musm musm deleted the patch-11 branch August 25, 2017 13:24
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.

6 participants