FiniteDifferences.jl icon indicating copy to clipboard operation
FiniteDifferences.jl copied to clipboard

Should `to_vec(::Integer)` return an empty vector

Open mzgubic opened this issue 4 years ago • 8 comments

Integers can't be perturbed, so their to_vec should be empty. Originally suggested in https://github.com/JuliaDiff/FiniteDifferences.jl/pull/187

function to_vec(x::Integer)
     Integer_from_vec(v) = x
     return Bool[], Integer_from_vec
end

Would also fix (a part of) https://github.com/chrisbrahms/Hankel.jl/pull/27

Did not test yet whether it messes up any existing ChainRules tests.

mzgubic avatar Aug 04 '21 08:08 mzgubic

This would make sense to me.

willtebbutt avatar Aug 04 '21 09:08 willtebbutt

The practical problem with this, is that sometimes people want to test special case methods that take integers. Which is just all round hard, since when finite diferencing they don't get integers.

But they might still want to test it against custom rules that do. So I wonder if we really need to handle this better in CRTU or somewhere in FiniteDifferences.

Right now we often (always?) error for it

oxinabox avatar Oct 01 '21 12:10 oxinabox

Could you provide an example @oxinabox ? I'm not really clear what sort of thing you're imagining.

willtebbutt avatar Oct 01 '21 12:10 willtebbutt

For example

function rrule(::typeof(sinpi), x)
    sinpi_pullback(dy) = -pi*cospi(x)
    foo(x), sinpi_pullback
end

function rrule(::typeof(sinpi), x::Integer)
    sinpi_pullback(dy) = Zero()
    foo(x), sinpi_pullback
end

oxinabox avatar Oct 01 '21 12:10 oxinabox

And what we're proposing to do here would solve this problem, yes?

willtebbutt avatar Oct 01 '21 12:10 willtebbutt

Would it? Cool!

oxinabox avatar Oct 01 '21 13:10 oxinabox

Isn't this example the thing which already works? Integer taken to mean non-differentiable:

julia> mysinpi(x) = sinpi(x)
mysinpi (generic function with 1 method)

julia> function ChainRulesCore.rrule(::typeof(mysinpi), x)
           sinpi_pullback(dy) = NoTangent(), pi*cospi(x)*dy
           sinpi(x), sinpi_pullback
       end

julia> function ChainRulesCore.rrule(::typeof(mysinpi), x::Integer)
           sinpi_pullback(dy) = NoTangent(), NoTangent()
           sinpi(x), sinpi_pullback
       end

julia> test_rrule(mysinpi, 1.23)
Test Summary:                  | Pass  Total
test_rrule: mysinpi on Float64 |    7      7
Test.DefaultTestSet("test_rrule: mysinpi on Float64", Any[], 7, false, false)

julia> test_rrule(mysinpi, 1)
Test Summary:                | Pass  Total
test_rrule: mysinpi on Int64 |    6      6
Test.DefaultTestSet("test_rrule: mysinpi on Int64", Any[], 6, false, false)

mcabbott avatar Oct 01 '21 13:10 mcabbott

Oh, yeah, this particular example is. I think stuff would need to be nested in order to break it.

willtebbutt avatar Oct 01 '21 14:10 willtebbutt