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.

Sign up for GitHub

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

irrationals: restrict assume effects annotations to known types #55886

Merged
merged 3 commits into from
Oct 10, 2024
Merged

irrationals: restrict assume effects annotations to known types #55886

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cc58d23
irrationals: restrict assume effects annotations to known types
nsajko Sep 26, 2024
5dd2a85
qualify `lessrational` in MathConstants
nsajko Sep 26, 2024
8bc317c
Merge branch 'master' into known_irrational
nsajko Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions base/irrationals.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ AbstractFloat(x::AbstractIrrational) = Float64(x)::Float64
Float16(x::AbstractIrrational) = Float16(Float32(x)::Float32)
Complex{T}(x::AbstractIrrational) where {T<:Real} = Complex{T}(T(x))

# XXX this may change `DEFAULT_PRECISION`, thus not effect free
@assume_effects :total function Rational{T}(x::AbstractIrrational) where T<:Integer
function _irrational_to_rational(::Type{T}, x::AbstractIrrational) where T<:Integer
o = precision(BigFloat)
p = 256
while true
Expand All @@ -66,13 +65,16 @@ Complex{T}(x::AbstractIrrational) where {T<:Real} = Complex{T}(T(x))
p += 32
end
end
Rational{BigInt}(x::AbstractIrrational) = throw(ArgumentError("Cannot convert an AbstractIrrational to a Rational{BigInt}: use rationalize(BigInt, x) instead"))
Rational{T}(x::AbstractIrrational) where {T<:Integer} = _irrational_to_rational(T, x)
_throw_argument_error_irrational_to_rational_bigint() = throw(ArgumentError("Cannot convert an AbstractIrrational to a Rational{BigInt}: use rationalize(BigInt, x) instead"))
Rational{BigInt}(::AbstractIrrational) = _throw_argument_error_irrational_to_rational_bigint()

@assume_effects :total function (t::Type{T})(x::AbstractIrrational, r::RoundingMode) where T<:Union{Float32,Float64}
function _irrational_to_float(::Type{T}, x::AbstractIrrational, r::RoundingMode) where T<:Union{Float32,Float64}
setprecision(BigFloat, 256) do
T(BigFloat(x)::BigFloat, r)
end
end
(::Type{T})(x::AbstractIrrational, r::RoundingMode) where {T<:Union{Float32,Float64}} = _irrational_to_float(T, x, r)

float(::Type{<:AbstractIrrational}) = Float64

Expand Down Expand Up @@ -110,14 +112,18 @@ end
<=(x::AbstractFloat, y::AbstractIrrational) = x < y

# Irrational vs Rational
@assume_effects :total function rationalize(::Type{T}, x::AbstractIrrational; tol::Real=0) where T
function _rationalize_irrational(::Type{T}, x::AbstractIrrational, tol::Real) where {T<:Integer}
return rationalize(T, big(x), tol=tol)
end
@assume_effects :total function lessrational(rx::Rational{<:Integer}, x::AbstractIrrational)
# an @assume_effects :total version of `<` for determining if the rationalization of
# an irrational number required rounding up or down
function rationalize(::Type{T}, x::AbstractIrrational; tol::Real=0) where {T<:Integer}
return _rationalize_irrational(T, x, tol)
end
function _lessrational(rx::Rational, x::AbstractIrrational)
return rx < big(x)
end
function lessrational(rx::Rational, x::AbstractIrrational)
return _lessrational(rx, x)
end
function <(x::AbstractIrrational, y::Rational{T}) where T
T <: Unsigned && x < 0.0 && return true
rx = rationalize(T, x)
Expand Down
20 changes: 20 additions & 0 deletions base/mathconstants.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,26 @@ Base.@irrational γ euler
Base.@irrational φ (1+sqrt(big(5)))/2
Base.@irrational catalan catalan

const _KnownIrrational = Union{
typeof(π), typeof(ℯ), typeof(γ), typeof(φ), typeof(catalan)
}

function Rational{BigInt}(::_KnownIrrational)
Base._throw_argument_error_irrational_to_rational_bigint()
end
Base.@assume_effects :foldable function Rational{T}(x::_KnownIrrational) where {T<:Integer}
Base._irrational_to_rational(T, x)
end
Base.@assume_effects :foldable function (::Type{T})(x::_KnownIrrational, r::RoundingMode) where {T<:Union{Float32,Float64}}
Base._irrational_to_float(T, x, r)
end
Base.@assume_effects :foldable function rationalize(::Type{T}, x::_KnownIrrational; tol::Real=0) where {T<:Integer}
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be Base.rationalize. Right now it creates a new function Base.MathConstants.rationalize. Consequently, Base.rationalize(Int, π) isn’t constant-folded in Julia 1.10 and 1.11:

julia> @btime rationalize(Int,π);
  1.837 ns (0 allocations: 0 bytes)      # v1.9: constant-folded
  88.416 μs (412 allocations: 15.00 KiB) # v1.10: not constant-folded

Copy link
Member

Choose a reason for hiding this comment

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

A comment easily gets lost so I think it is preferable to create a new issue where it can be given a milestone for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

I opened a PR: #56793

Base._rationalize_irrational(T, x, tol)
end
Base.@assume_effects :foldable function lessrational(rx::Rational, x::_KnownIrrational)
nsajko marked this conversation as resolved.
Show resolved Hide resolved
Base._lessrational(rx, x)
end

# aliases
"""
π
Expand Down