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

isapprox (≈) returns different answers depending on the abstract element type #1083

Open
mbauman opened this issue Aug 2, 2024 · 3 comments
Labels
correctness bug ⚠ Bugs that are likely to lead to incorrect results in user code without throwing

Comments

@mbauman
Copy link
Member

mbauman commented Aug 2, 2024

This is quite problematic:

julia> Any[824.9999999999999] ≈ [825]
true

julia> Number[824.9999999999999] ≈ [825]
false

julia> Union{Number,Missing}[824.9999999999999] ≈ [825]
true

julia> Real[824.9999999999999] ≈ [825]
false

julia> Union{Real,Missing}[824.9999999999999] ≈ [825]
true

julia> AbstractFloat[824.9999999999999] ≈ [825]
ERROR: MethodError: no method matching eps(::Type{AbstractFloat})
The function `eps` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  eps(::Type{BigFloat})
   @ Base mpfr.jl:1087
  eps(::Type{Float32})
   @ Base float.jl:1049
  eps(::Type{Float64})
   @ Base float.jl:1050
  ...

Stacktrace:
 [1] rtoldefault(::Type{AbstractFloat})
   @ Base ./floatfuncs.jl:260
 [2] rtoldefault(x::Type{AbstractFloat}, y::Type{Int64}, atol::Int64)
   @ Base ./floatfuncs.jl:263
 [3] isapprox(x::Vector{AbstractFloat}, y::Vector{Int64})
   @ LinearAlgebra ~/Projects/Julia/julia/usr/share/julia/stdlib/v1.12/LinearAlgebra/src/generic.jl:1880
 [4] top-level scope
   @ REPL[20]:1

julia> Union{AbstractFloat,Missing}[824.9999999999999] ≈ [825]
true

julia> Union{AbstractFloat,Int,}[824.9999999999999] ≈ [825]
false

julia> Union{Float32, Float64}[824.9999999999999] ≈ [825]
ERROR: MethodError: no method matching eps(::Type{Union{Float32, Float64}})

Closest candidates are:
  eps(::Type{BigFloat})
   @ Base mpfr.jl:1013
  eps(::Type{Dates.Time})
   @ Dates ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/Dates/src/types.jl:434
  eps(::Type{Float16})
   @ Base float.jl:967
  ...

Stacktrace:
 [1] rtoldefault(::Type{Union{Float32, Float64}})
   @ Base ./floatfuncs.jl:344
 [2] rtoldefault(x::Type{Union{Float32, Float64}}, y::Type{Int64}, atol::Int64)
   @ Base ./floatfuncs.jl:347
 [3] isapprox(x::Vector{Union{Float32, Float64}}, y::Vector{Int64})
   @ LinearAlgebra ~/.julia/juliaup/julia-1.10.4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/LinearAlgebra/src/generic.jl:1785
 [4] top-level scope
   @ REPL[22]:1

julia> Union{Int, Float64}[824.9999999999999] ≈ [825]
false
@mbauman mbauman added the correctness bug ⚠ Bugs that are likely to lead to incorrect results in user code without throwing label Aug 2, 2024
@Moelf
Copy link
Contributor

Moelf commented Aug 2, 2024

  • should all of these not error?
  • should all of these return true?

@KlausC
Copy link
Contributor

KlausC commented Aug 3, 2024

That can be attributed IMO to LinearAlgebra.promote_leaf_types:

julia> LinearAlgebra.promote_leaf_eltypes(Any[123.4])
Float64

julia> LinearAlgebra.promote_leaf_eltypes(Number[123.4])
Number

That should return Float64 in both cases. As a consequence, isapprox would and should return true in all cases.

@DanielVandH
Copy link

That can be attributed IMO to LinearAlgebra.promote_leaf_types:

julia> LinearAlgebra.promote_leaf_eltypes(Any[123.4])
Float64

julia> LinearAlgebra.promote_leaf_eltypes(Number[123.4])
Number

That should return Float64 in both cases. As a consequence, isapprox would and should return true in all cases.

The first two methods in

promote_leaf_eltypes(x::Union{AbstractArray{T},Tuple{T,Vararg{T}}}) where {T<:Number} = T
promote_leaf_eltypes(x::Union{AbstractArray{T},Tuple{T,Vararg{T}}}) where {T<:NumberArray} = eltype(T)
promote_leaf_eltypes(x::T) where {T} = T
promote_leaf_eltypes(x::Union{AbstractArray,Tuple}) = mapreduce(promote_leaf_eltypes, promote_type, x; init=Bool)

are probably not ideal when T is not a concrete type, since T <: Number for e.g. Number, Real, Complex, and AbstractFloat. In these cases they don't match the definition in the last line, or the documented behaviour of this function being the same as promote_type(typeof(leaf1), typeof(leaf2), ...). Perhaps these methods need to be updated to account for this case.

@KristofferC KristofferC transferred this issue from JuliaLang/julia Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
correctness bug ⚠ Bugs that are likely to lead to incorrect results in user code without throwing
Projects
None yet
Development

No branches or pull requests

4 participants