From cc8f0cd07b7ea05d06f60bc1fd485cd3b8aad40b Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Tue, 27 Jul 2021 11:52:44 +0100 Subject: [PATCH 1/4] Don't try and convert to FloatX except if Integer or AbstractFloat --- Project.toml | 2 +- src/projection.jl | 37 ++++++++++++++++++++++++++++++++++--- test/projection.jl | 25 ++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/Project.toml b/Project.toml index d6720e438..7791f2ca6 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "ChainRulesCore" uuid = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4" -version = "1.0.1" +version = "1.0.2" [deps] Compat = "34da2185-b29b-5c13-b0c7-acf172513d20" diff --git a/src/projection.jl b/src/projection.jl index 7fb968fb9..f67af54af 100644 --- a/src/projection.jl +++ b/src/projection.jl @@ -135,13 +135,44 @@ ProjectTo(::Real) = ProjectTo{Real}() ProjectTo(::Complex) = ProjectTo{Complex}() ProjectTo(::Number) = ProjectTo{Number}() for T in (Float16, Float32, Float64, ComplexF16, ComplexF32, ComplexF64) - # Preserve low-precision floats as accidental promotion is a common perforance bug + # Preserve low-precision floats as accidental promotion is a common perforance bug @eval ProjectTo(::$T) = ProjectTo{$T}() end ProjectTo(x::Integer) = ProjectTo(float(x)) ProjectTo(x::Complex{<:Integer}) = ProjectTo(float(x)) -(::ProjectTo{T})(dx::Number) where {T<:Number} = convert(T, dx) -(::ProjectTo{T})(dx::Number) where {T<:Real} = convert(T, real(dx)) + +# Preserve low-precision floats as accidental promotion is a common perforance bug +(::ProjectTo{T})(dx::AbstractFloat) where T<:AbstractFloat = convert(T, dx) +(::ProjectTo{T})(dx::Integer) where T<:AbstractFloat = convert(T, dx) + + +# We asked for a number/real and they gave use one. We did ask for a particular concrete +# type, but that is just for the preserving low precision floats, which is handled above. +# Any Number/Real actually occupies the same subspace, so we can trust them. +# In particular, this makes weirder Real subtypes that are not simply the values like +# ForwardDiff.Dual and Symbolics.Sym work, because we stay out of their way. +(::ProjectTo{<:Number})(dx::Number) where {T<:Number} = dx +(::ProjectTo{<:Real})(dx::Real) = dx + +(::ProjectTo{T})(dx::Complex) where T<:Real = ProjectTo(zero(T))(real(dx)) + +# Complex +function (proj::ProjectTo{<:Complex{<:AbstractFloat}})( + dx::Complex{<:Union{AbstractFloat,Integer}} +) + # in this case we can just convert as we know we are dealing with + # boring floating point types or integers + return convert(project_type(proj), dx) +end +# Pass though non-AbstractFloat to project each component +function (::ProjectTo{<:Complex{T}})(dx::Complex) where T + project = ProjectTo(zero(T)) + return Complex(project(real(dx)), project(imag(dx))) +end +function (::ProjectTo{<:Complex{T}})(dx::Real) where T + project = ProjectTo(zero(T)) + return Complex(project(dx), project(zero(dx))) +end # Arrays # If we don't have a more specialized `ProjectTo` rule, we just assume that there is diff --git a/test/projection.jl b/test/projection.jl index 209542781..f80c8f2c0 100644 --- a/test/projection.jl +++ b/test/projection.jl @@ -2,6 +2,15 @@ using ChainRulesCore, Test using LinearAlgebra, SparseArrays using OffsetArrays, BenchmarkTools +# Like ForwardDiff.jl's Dual +struct Dual{T<:Real} <: Real + value::T + partial::T +end +Base.real(x::Dual) = x +Base.float(x::Dual) = Dual(float(x.value), float(x.partial)) +Base.zero(x::Dual) = Dual(zero(x.value), zero(x.partial)) + @testset "projection" begin ##### @@ -12,14 +21,28 @@ using OffsetArrays, BenchmarkTools # real / complex @test ProjectTo(1.0)(2.0 + 3im) === 2.0 @test ProjectTo(1.0 + 2.0im)(3.0) === 3.0 + 0.0im + @test ProjectTo(2.0+3.0im)(1+1im) === 1.0+1.0im + @test ProjectTo(2.0)(1+1im) === 1.0 + # storage - @test ProjectTo(1)(pi) === Float64(pi) + @test ProjectTo(1)(pi) === pi @test ProjectTo(1 + im)(pi) === ComplexF64(pi) @test ProjectTo(1//2)(3//4) === 3//4 @test ProjectTo(1.0f0)(1 / 2) === 0.5f0 @test ProjectTo(1.0f0 + 2im)(3) === 3.0f0 + 0im @test ProjectTo(big(1.0))(2) === 2 + @test ProjectTo(1.0)(2) === 2.0 + end + + @testset "Dual" begin # some weird Real subtype that we should basically leave alone + @test ProjectTo(1.0)(Dual(1.0, 2.0)) isa Dual + @test ProjectTo(1.0)(Dual(1, 2)) isa Dual + @test ProjectTo(1.0 + 1im)(Dual(1.0, 2.0)) isa Complex{<:Dual} + @test ProjectTo(1.0 + 1im)( + Complex(Dual(1.0, 2.0), Dual(1.0, 2.0)) + ) isa Complex{<:Dual} + @test ProjectTo(1.0)(Complex(Dual(1.0, 2.0), Dual(1.0, 2.0))) isa Dual end @testset "Base: arrays of numbers" begin From 50a1156751de033a50387ce87e20f50391d53caa Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Tue, 27 Jul 2021 13:41:50 +0100 Subject: [PATCH 2/4] clean up code --- src/projection.jl | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/projection.jl b/src/projection.jl index f67af54af..f8bf31243 100644 --- a/src/projection.jl +++ b/src/projection.jl @@ -135,44 +135,41 @@ ProjectTo(::Real) = ProjectTo{Real}() ProjectTo(::Complex) = ProjectTo{Complex}() ProjectTo(::Number) = ProjectTo{Number}() for T in (Float16, Float32, Float64, ComplexF16, ComplexF32, ComplexF64) - # Preserve low-precision floats as accidental promotion is a common perforance bug + # Preserve low-precision floats as accidental promotion is a common performance bug @eval ProjectTo(::$T) = ProjectTo{$T}() end ProjectTo(x::Integer) = ProjectTo(float(x)) ProjectTo(x::Complex{<:Integer}) = ProjectTo(float(x)) -# Preserve low-precision floats as accidental promotion is a common perforance bug +# Preserve low-precision floats as accidental promotion is a common performance bug +# In these cases we can just `convert` as we know we are dealing with plain and simple types (::ProjectTo{T})(dx::AbstractFloat) where T<:AbstractFloat = convert(T, dx) -(::ProjectTo{T})(dx::Integer) where T<:AbstractFloat = convert(T, dx) - +(::ProjectTo{T})(dx::Integer) where T<:AbstractFloat = convert(T, dx) #needed to avoid ambiguity # We asked for a number/real and they gave use one. We did ask for a particular concrete # type, but that is just for the preserving low precision floats, which is handled above. # Any Number/Real actually occupies the same subspace, so we can trust them. # In particular, this makes weirder Real subtypes that are not simply the values like # ForwardDiff.Dual and Symbolics.Sym work, because we stay out of their way. -(::ProjectTo{<:Number})(dx::Number) where {T<:Number} = dx -(::ProjectTo{<:Real})(dx::Real) = dx +(::ProjectTo{<:Number})(dx::Number) = dx +# If you remove the above julia sometimes can't find the (::ProjectTo{T})(::T) for complex T +# Seems like it might be a julia bug? -(::ProjectTo{T})(dx::Complex) where T<:Real = ProjectTo(zero(T))(real(dx)) +(project::ProjectTo{<:Real})(dx::Complex) = project(real(dx)) # Complex -function (proj::ProjectTo{<:Complex{<:AbstractFloat}})( - dx::Complex{<:Union{AbstractFloat,Integer}} -) - # in this case we can just convert as we know we are dealing with - # boring floating point types or integers - return convert(project_type(proj), dx) -end -# Pass though non-AbstractFloat to project each component + +# Preserve low-precision floats as accidental promotion is a common performance bug +# In these cases we can just `convert` as we know we are dealing with plain and simple types +(::ProjectTo{T})(dx::Complex{<:AbstractFloat}) where {T<:Complex{<:AbstractFloat}} = convert(T, dx) +(::ProjectTo{T})(dx::AbstractFloat) where {T<:Complex{<:AbstractFloat}} = convert(T, dx) + +# For on-AbstractFloat other types pass though to project each component function (::ProjectTo{<:Complex{T}})(dx::Complex) where T project = ProjectTo(zero(T)) return Complex(project(real(dx)), project(imag(dx))) end -function (::ProjectTo{<:Complex{T}})(dx::Real) where T - project = ProjectTo(zero(T)) - return Complex(project(dx), project(zero(dx))) -end +(::ProjectTo{<:Complex{T}})(dx::Real) where T = Complex(ProjectTo(zero(T))(dx)) # Arrays # If we don't have a more specialized `ProjectTo` rule, we just assume that there is From d324782fce0dcdfce652d875f9dff6dfe4b5ed71 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Tue, 27 Jul 2021 14:33:27 +0100 Subject: [PATCH 3/4] Shorten explination --- src/projection.jl | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/projection.jl b/src/projection.jl index f8bf31243..648db6dfa 100644 --- a/src/projection.jl +++ b/src/projection.jl @@ -146,14 +146,11 @@ ProjectTo(x::Complex{<:Integer}) = ProjectTo(float(x)) (::ProjectTo{T})(dx::AbstractFloat) where T<:AbstractFloat = convert(T, dx) (::ProjectTo{T})(dx::Integer) where T<:AbstractFloat = convert(T, dx) #needed to avoid ambiguity -# We asked for a number/real and they gave use one. We did ask for a particular concrete -# type, but that is just for the preserving low precision floats, which is handled above. -# Any Number/Real actually occupies the same subspace, so we can trust them. -# In particular, this makes weirder Real subtypes that are not simply the values like -# ForwardDiff.Dual and Symbolics.Sym work, because we stay out of their way. +# Other numbers, including e.g. ForwardDiff.Dual and Symbolics.Sym, should pass through. +# We assume (lacking evidence to the contrary) that +# The (::ProjectTo{T})(::T) method doesn't work because we are allowing a different +# Number type that might not be a subtype of the `project_type`. (::ProjectTo{<:Number})(dx::Number) = dx -# If you remove the above julia sometimes can't find the (::ProjectTo{T})(::T) for complex T -# Seems like it might be a julia bug? (project::ProjectTo{<:Real})(dx::Complex) = project(real(dx)) From 127bae5e6b50647f09e5bca3301e504ed13bbd6b Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Tue, 27 Jul 2021 14:46:56 +0100 Subject: [PATCH 4/4] remove recursive complex support, and reorder --- src/projection.jl | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/projection.jl b/src/projection.jl index 648db6dfa..a5f89d025 100644 --- a/src/projection.jl +++ b/src/projection.jl @@ -134,39 +134,32 @@ ProjectTo(::Bool) = ProjectTo{NoTangent}() # same projector as ProjectTo(::Abst ProjectTo(::Real) = ProjectTo{Real}() ProjectTo(::Complex) = ProjectTo{Complex}() ProjectTo(::Number) = ProjectTo{Number}() -for T in (Float16, Float32, Float64, ComplexF16, ComplexF32, ComplexF64) - # Preserve low-precision floats as accidental promotion is a common performance bug - @eval ProjectTo(::$T) = ProjectTo{$T}() -end + ProjectTo(x::Integer) = ProjectTo(float(x)) ProjectTo(x::Complex{<:Integer}) = ProjectTo(float(x)) # Preserve low-precision floats as accidental promotion is a common performance bug +for T in (Float16, Float32, Float64, ComplexF16, ComplexF32, ComplexF64) + @eval ProjectTo(::$T) = ProjectTo{$T}() +end + # In these cases we can just `convert` as we know we are dealing with plain and simple types (::ProjectTo{T})(dx::AbstractFloat) where T<:AbstractFloat = convert(T, dx) (::ProjectTo{T})(dx::Integer) where T<:AbstractFloat = convert(T, dx) #needed to avoid ambiguity +# simple Complex{<:AbstractFloat}} cases +(::ProjectTo{T})(dx::Complex{<:AbstractFloat}) where {T<:Complex{<:AbstractFloat}} = convert(T, dx) +(::ProjectTo{T})(dx::AbstractFloat) where {T<:Complex{<:AbstractFloat}} = convert(T, dx) +(::ProjectTo{T})(dx::Complex{<:Integer}) where {T<:Complex{<:AbstractFloat}} = convert(T, dx) +(::ProjectTo{T})(dx::Integer) where {T<:Complex{<:AbstractFloat}} = convert(T, dx) # Other numbers, including e.g. ForwardDiff.Dual and Symbolics.Sym, should pass through. -# We assume (lacking evidence to the contrary) that +# We assume (lacking evidence to the contrary) that it is the right subspace of numebers # The (::ProjectTo{T})(::T) method doesn't work because we are allowing a different # Number type that might not be a subtype of the `project_type`. (::ProjectTo{<:Number})(dx::Number) = dx (project::ProjectTo{<:Real})(dx::Complex) = project(real(dx)) - -# Complex - -# Preserve low-precision floats as accidental promotion is a common performance bug -# In these cases we can just `convert` as we know we are dealing with plain and simple types -(::ProjectTo{T})(dx::Complex{<:AbstractFloat}) where {T<:Complex{<:AbstractFloat}} = convert(T, dx) -(::ProjectTo{T})(dx::AbstractFloat) where {T<:Complex{<:AbstractFloat}} = convert(T, dx) - -# For on-AbstractFloat other types pass though to project each component -function (::ProjectTo{<:Complex{T}})(dx::Complex) where T - project = ProjectTo(zero(T)) - return Complex(project(real(dx)), project(imag(dx))) -end -(::ProjectTo{<:Complex{T}})(dx::Real) where T = Complex(ProjectTo(zero(T))(dx)) +(project::ProjectTo{<:Complex})(dx::Real) = project(complex(dx)) # Arrays # If we don't have a more specialized `ProjectTo` rule, we just assume that there is