isapprox: test max(atol,rtol*...) rather than atol+rtol*... (#22742)

* isapprox now tests max(atol,rtol*...) rather than atol+rtol*...

* move news to breaking section

* in isapprox, make default rtol=0 if atol>0 is specified

* consistent isapprox for UniformScaling

* whoops, remove old code

* deprecate 2-arg rtoldefault

Author: stevengj

NEWS.md

@@ -120,6 +120,10 @@ This section lists changes that do not have deprecation warnings.
     `Bidiagonal{T,V<:AbstractVector{T}}` and `SymTridiagonal{T,V<:AbstractVector{T}}`
     respectively ([#22718], [#22925], [#23035]).
 
+  * `isapprox(x,y)` now tests `norm(x-y) <= max(atol, rtol*max(norm(x), norm(y)))`
+    rather than `norm(x-y) <= atol + ...`, and `rtol` defaults to zero
+    if an `atol > 0` is specified ([#22742]).
+
   * Spaces are no longer allowed between `@` and the name of a macro in a macro call ([#22868]).
 
   * Juxtaposition of a non-literal with a macro call (`x@macro`) is no longer valid syntax ([#22868]).

base/deprecated.jl

@@ -1661,6 +1661,8 @@ end
 @deprecate num2hex(x::Union{Float16,Float32,Float64}) hex(reintepret(Unsigned, x), sizeof(x)*2)
 @deprecate num2hex(n::Integer) hex(n, sizeof(n)*2)
 
+# PR #22742: change in isapprox semantics
+@deprecate rtoldefault(x,y) rtoldefault(x,y,0) false
 
 # END 0.7 deprecations
 

base/floatfuncs.jl

@@ -189,15 +189,16 @@ end
 
 # isapprox: approximate equality of numbers
 """
-    isapprox(x, y; rtol::Real=sqrt(eps), atol::Real=0, nans::Bool=false, norm::Function)
+    isapprox(x, y; rtol::Real=atol>0 ? √eps : 0, atol::Real=0, nans::Bool=false, norm::Function)
 
-Inexact equality comparison: `true` if `norm(x-y) <= atol + rtol*max(norm(x), norm(y))`. The
+Inexact equality comparison: `true` if `norm(x-y) <= max(atol, rtol*max(norm(x), norm(y)))`. The
 default `atol` is zero and the default `rtol` depends on the types of `x` and `y`. The keyword
 argument `nans` determines whether or not NaN values are considered equal (defaults to false).
 
-For real or complex floating-point values, `rtol` defaults to
-`sqrt(eps(typeof(real(x-y))))`. This corresponds to requiring equality of about half of the
-significand digits. For other types, `rtol` defaults to zero.
+For real or complex floating-point values, if an `atol > 0` is not specified, `rtol` defaults to
+the square root of [`eps`](@ref) of the type of `x` or `y`, whichever is bigger (least precise).
+This corresponds to requiring equality of about half of the significand digits. Otherwise,
+e.g. for integer arguments or if an `atol > 0` is supplied, `rtol` defaults to zero.
 
 `x` and `y` may also be arrays of numbers, in which case `norm` defaults to `vecnorm` but
 may be changed by passing a `norm::Function` keyword argument. (For numbers, `norm` is the
@@ -220,8 +221,8 @@ julia> isapprox([10.0^9, 1.0], [10.0^9, 2.0])
 true
 ```
 """
-function isapprox(x::Number, y::Number; rtol::Real=rtoldefault(x,y), atol::Real=0, nans::Bool=false)
-    x == y || (isfinite(x) && isfinite(y) && abs(x-y) <= atol + rtol*max(abs(x), abs(y))) || (nans && isnan(x) && isnan(y))
+function isapprox(x::Number, y::Number; atol::Real=0, rtol::Real=rtoldefault(x,y,atol), nans::Bool=false)
+    x == y || (isfinite(x) && isfinite(y) && abs(x-y) <= max(atol, rtol*max(abs(x), abs(y)))) || (nans && isnan(x) && isnan(y))
 end
 
 const ≈ = isapprox
@@ -230,7 +231,10 @@ const ≈ = isapprox
 # default tolerance arguments
 rtoldefault(::Type{T}) where {T<:AbstractFloat} = sqrt(eps(T))
 rtoldefault(::Type{<:Real}) = 0
-rtoldefault(x::Union{T,Type{T}}, y::Union{S,Type{S}}) where {T<:Number,S<:Number} = max(rtoldefault(real(T)),rtoldefault(real(S)))
+function rtoldefault(x::Union{T,Type{T}}, y::Union{S,Type{S}}, atol::Real) where {T<:Number,S<:Number}
+    rtol = max(rtoldefault(real(T)),rtoldefault(real(S)))
+    return atol > 0 ? zero(rtol) : rtol
+end
 
 # fused multiply-add
 fma_libm(x::Float32, y::Float32, z::Float32) =

base/linalg/generic.jl

@@ -1296,11 +1296,12 @@ promote_leaf_eltypes(x::Union{AbstractArray,Tuple}) = mapreduce(promote_leaf_elt
 # Supports nested arrays; e.g., for `a = [[1,2, [3,4]], 5.0, [6im, [7.0, 8.0]]]`
 # `a ≈ a` is `true`.
 function isapprox(x::AbstractArray, y::AbstractArray;
-    rtol::Real=Base.rtoldefault(promote_leaf_eltypes(x),promote_leaf_eltypes(y)),
-    atol::Real=0, nans::Bool=false, norm::Function=vecnorm)
+    atol::Real=0,
+    rtol::Real=Base.rtoldefault(promote_leaf_eltypes(x),promote_leaf_eltypes(y),atol),
+    nans::Bool=false, norm::Function=vecnorm)
     d = norm(x - y)
     if isfinite(d)
-        return d <= atol + rtol*max(norm(x), norm(y))
+        return d <= max(atol, rtol*max(norm(x), norm(y)))
     else
         # Fall back to a component-wise approximate comparison
         return all(ab -> isapprox(ab[1], ab[2]; rtol=rtol, atol=atol, nans=nans), zip(x, y))

base/linalg/uniformscaling.jl

@@ -197,15 +197,16 @@ broadcast(::typeof(/), J::UniformScaling,x::Number) = UniformScaling(J.λ/x)
 ==(J1::UniformScaling,J2::UniformScaling) = (J1.λ == J2.λ)
 
 function isapprox(J1::UniformScaling{T}, J2::UniformScaling{S};
-            rtol::Real=Base.rtoldefault(T,S), atol::Real=0, nans::Bool=false) where {T<:Number,S<:Number}
+            atol::Real=0, rtol::Real=Base.rtoldefault(T,S,atol), nans::Bool=false) where {T<:Number,S<:Number}
     isapprox(J1.λ, J2.λ, rtol=rtol, atol=atol, nans=nans)
 end
 function isapprox(J::UniformScaling,A::AbstractMatrix;
-                  rtol::Real=rtoldefault(promote_leaf_eltypes(A),eltype(J)),
-                  atol::Real=0, nans::Bool=false, norm::Function=vecnorm)
+                  atol::Real=0,
+                  rtol::Real=rtoldefault(promote_leaf_eltypes(A),eltype(J),atol),
+                  nans::Bool=false, norm::Function=vecnorm)
     n = checksquare(A)
     Jnorm = norm === vecnorm ? abs(J.λ)*sqrt(n) : (norm === Base.norm ? abs(J.λ) : norm(diagm(fill(J.λ, n))))
-    return norm(A - J) <= atol + rtol*max(norm(A), Jnorm)
+    return norm(A - J) <= max(atol, rtol*max(norm(A), Jnorm))
 end
 isapprox(A::AbstractMatrix,J::UniformScaling;kwargs...) = isapprox(J,A;kwargs...)
 
@@ -335,4 +336,3 @@ UniformScaling{Float64}
 ```
 """
 chol(J::UniformScaling, args...) = ((C, info) = _chol!(J, nothing); @assertposdef C info)
-

test/math.jl

@@ -669,6 +669,11 @@ end
     @test exp10(Float16(1.0)) === Float16(exp10(1.0))
 end
 
+# #22742: updated isapprox semantics
+@test !isapprox(1.0, 1.0+1e-12, atol=1e-14)
+@test isapprox(1.0, 1.0+0.5*sqrt(eps(1.0)))
+@test !isapprox(1.0, 1.0+1.5*sqrt(eps(1.0)), atol=sqrt(eps(1.0)))
+
 # test AbstractFloat fallback pr22716
 struct Float22716{T<:AbstractFloat} <: AbstractFloat
     x::T