-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add option to check if frules and rrules are type-inferrable #78
Conversation
Hmmm I'm stumped by this one -- not a massive |
We have a scary, but functional tool for this in Basically do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than missing the unhappy path tests, this seems good.
It is a breaking change, since it defaults to true, but I think that is fine?
Yes, I think we do want it to be on by default. It is already exposing a fair amount of type-instability in ChainRules. It is a breaking change, and it needs a bump to minor version number. |
This is ready to merge, but because it bumps the minor version number and will require some changes to ChainRules to bring back compatibility, maybe I should wait for other non-breaking PRs to be merged first? |
I have no objection to it being merged, but @oxinabox might want to get a couple of his things in first? |
This can be merged now |
As discussed with @willtebbutt in JuliaDiff/ChainRules.jl#321 (comment), this PR adds a
check_inferred=true
default keyword argument to the tester functions to check if the frules, rrules, pullbacks, and thunks are all type-stable.The functionality is there, but I'm having some difficulty testing that
test_scalar
fails when anrrule
orfrule
is type-unstable. Perhaps a testing kung fu master could give me a pointer.