Add convert for AbstractZero #252
Conversation
|
Not sure why the tests for ChainRules fail, they work locally for me |
willtebbutt
left a comment
willtebbutt
left a comment
There was a problem hiding this comment.
LGTM. Happy for this to be merged once tests pass.
src/differentials/abstract_zero.jl
Outdated
| Base.transpose(z::AbstractZero) = z | ||
| Base.:/(z::AbstractZero, ::Any) = z | ||
|
|
||
| Base.convert(::Type{T}, x::AbstractZero) where T <: Real = zero(T) |
There was a problem hiding this comment.
Any reason to restrict this to Real? I think it would make sense to at least allow Complex, but all subtypes of Number should work as well here.
There was a problem hiding this comment.
Oh, and I don't know if we really want to implement this for all AbstractZeros. I am not sure this makes sense for DoesNotExist(), so it might be better to do this just for Zero().
There was a problem hiding this comment.
Thanks for catching the Real -> Number, it was just an error on my part.
My understanding of the Zero vs DoesNotExist differential types is that the purpose of the distinction between them is primarily to signal meaning when defining rules. We do define mathematical operations on DoesNotExist in which it behaves like a zero, so it seems we aren't introducing any ideologically new behaviour by allowing it to be converted to a zero?
There was a problem hiding this comment.
| Base.convert(::Type{T}, x::AbstractZero) where T <: Real = zero(T) | |
| Base.convert(::Type{T}, x::AbstractZero) where T <: Number = zero(T) |
Helps with #112