Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opting out of rules #377

Closed
mzgubic opened this issue Jun 21, 2021 · 4 comments · Fixed by #398
Closed

Opting out of rules #377

mzgubic opened this issue Jun 21, 2021 · 4 comments · Fixed by #398
Labels
design Requires some desgin before changes are made
Milestone

Comments

@mzgubic
Copy link
Member

mzgubic commented Jun 21, 2021

The conclusion of the discussion spread across #343, JuliaDiff/ChainRules.jl#337, #347, and JuliaDiff/ChainRules.jl#232 (and possibly elsewhere), has been to prefer writing abstractly typed rules.

Two shortcomings of this approach are that

  1. In some cases, such as when the primal function is specialised for a subtype (e.g. Diagonal<:AbstractArray), the fallback rrule returning a Matrix is not correct. Solving that is the topic of Projecting Cotangents #286
  2. In other cases, using AD to differentiate through the specialised method is faster than falling back on the generic rule. For these, we need a way to opt out of using the generic fallback rule, and let AD compute the backward pass. Solving that is the topic of this issue.

There are a number of possible ways we could facilitate opting-out:

  • Option A) Return nothing. This has been tried in Take nothing seriously FluxML/Zygote.jl#967 but advised against in Use ChainRules RuleConfig FluxML/Zygote.jl#990 (comment) for Zygote. According to @oxinabox this would work in Diffractor, but not in Nabla? (please just edit this if that's wrong)
  • Option B) Have a no_rrule function that would be used to designate a particular signature does not have a rule.
  • Option C) Have a @no_rrule macro which would take a signature and generate code which returns nothing (as above), and automatically add the method to an internal list, which would be accessed by Zygote and Nabla.

Have I missed another option we've discussed?

@mzgubic mzgubic added the design Requires some desgin before changes are made label Jun 21, 2021
@mzgubic mzgubic added this to the v1 milestone Jun 21, 2021
@oxinabox
Copy link
Member

oxinabox commented Jun 21, 2021

Option A-2)
The first version of the ChainRules support in Zygote was Zygote#291.
This checked the return from rrule at runtime, rather than looking up the MethodTable in the generated function.
While I say run-time, it should have compiled that branch out of existance by type-inferring whether or not Nothing would be returned, and thus specializing away the check (til naturally invalidated by a rule being added).
A problem with it was that Zygote became super-slow, because it didn't type-infer and specialize away the branch.
because Zygote and type-inference are not friends.
But I think it might work if rather than using a if it used dispatch on the return value from rrule.
That might specialize properly, into a static dispatch.

cc @Keno does any of the options above work worse for Diffractor than current setup?
From some past discussions my impression is returning nothing is good for Diffractor, right?

@Keno
Copy link
Contributor

Keno commented Jun 21, 2021 via email

@oxinabox
Copy link
Member

So Diffractor does something like my Option A-2), I guess?

@Keno
Copy link
Contributor

Keno commented Jun 21, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Requires some desgin before changes are made
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants