Bug in rrule(*)

[dpsanders]

If I understand correctly, the following MWE should return numbers from the pullback:

julia> using ChainRules

julia> y = 3.0

julia> a, a_pullback = rrule(*, 2, y);

julia> a_pullback(1.0)
(Zero(), Thunk(ChainRules.var"#115#159"{Float64,Float64}(1.0, 3.0))
, Thunk(ChainRules.var"#116#160"{Float64,Int64}(1.0, 2))
)

[ettersi]

Why do you think these should be numbers? If you have a fancy number type where multiplication is expensive, not easily elidible and you only want one of the two pullbacks, then you need thunks to avoid unnecessary computations.

[dpsanders]

For the following reasons:

1. By analogy with other arithmetic operations:

julia> a, a_pullback = rrule(+, 1, 2);

julia> a_pullback(1)
(Zero(), 1, 1)

2. I do not want to have to special-case my generic code.

3. Julia knows which types I'm using and that they are not fancy.

4. Any fancy behaviour should be opt-in and controlled by a trait.

5. I didn't see anything in the docs warning that * was special and showing how to use it. (Although I didn't read all the docs.)

[dpsanders]

Apparently I just need to add unthunk(...) to the things returned from calling a pullback.
This should be added to the docs somewhere on the front page.

[ettersi]

Yes, unthunk() should be called on return values from frule and rrule.

I agree that the documentation could be better. My (by no means expert!) understanding is that this package is still somewhat in the prototyping stage, hence there has not been much of a push yet to really polish the documentation.

[oxinabox]

I agree that the documentation could be better. My (by no means expert!) understanding is that this package is still somewhat in the prototyping stage, hence there has not been much of a push yet to really polish the documentation.

We are trying really hard with the documentation.
Most of what we have is fairly polished,
but there are random gaps that are just absent,
and explaining thunks properly is one of them

This should be added to the docs somewhere on the front page.

There are many things that need to be added prominantly.
Its a hard take to balance.
Particularly since you want simple example early,
and adding mention of thunking too soon just complciates matters.
But I agree we could definately do better on this.

[nickrobinson251]

regarding docs for Thunk in general, it'd be good to have specific proposals for improvements :)
currently

• they're mentioned in the intro https://www.juliadiff.org/ChainRulesCore.jl/dev/index.html#Differentials
• we have a section on how to use them when writing rules https://www.juliadiff.org/ChainRulesCore.jl/dev/writing_good_rules.html#Use-Thunks-appropriately
• docstring with examples and answers to common questions https://www.juliadiff.org/ChainRulesCore.jl/dev/api.html#ChainRulesCore.Thunk

I'm sure we can improve things, but the more concrete the suggestion the better :)

I just need to add unthunk(...) to the things returned from calling a pullback. This should be added to the docs somewhere

yeah, that's a good suggestion. It looks like i made an issue for this (JuliaDiff/ChainRulesCore.jl#154) but it's not yet addressed (and since then the example has changed so we no longer show this, nevermind comment on it)