Don't return opted out rules. CRC 1.0 compat #8
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8 +/- ##
==========================================
- Coverage 96.29% 90.00% -6.30%
==========================================
Files 2 2
Lines 54 60 +6
==========================================
+ Hits 52 54 +2
- Misses 2 6 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
oxinabox
changed the title
| opted_out = Set(arg_type_tuple(m.sig) for m in _no_rule_list(rule_kind)) | ||
| return Iterators.filter(methods(rule_kind)) do m | ||
| return !_is_fallback(rule_kind, m) && !_requires_config(m) | ||
| return !_requires_config(m) && arg_type_tuple(m.sig) ∉ opted_out |
There was a problem hiding this comment.
Does this still exclude fallback rules? is it because _no_rule_list contains the same fallback signature as the rules? Is that not quite fragile?
There was a problem hiding this comment.
Conceptually CRC should have @opt_out rrule(::Any, :Any...)
It doesn't actually have that code, because that one it has has Keno's kwarg optimisation.
I would not be surprised if we roll the optimisation out for all @opt_out.
If we do it literally will have that opt out.
There was a problem hiding this comment.
hmm, so what is the current mechanism to exclude the fallback rule from appearing in this list? (given that we have removed !_is_fallback condition?)
Edit: ah, I see, the no_rrule has this signature in CRC
Partner to JuliaDiff/ChainRulesCore.jl#398
This does not help one actually opt out of rules.
It doesn't expost an API for keeping track of that.
(But I think maybe we can exactly copy the one for frule/rrule?)
That can be a follow up PR.
It is less pressing
What it does do is make sure the rules that return nothing are not incuded in the list of rules that get generated.
Which means that opting out will not cause e,g. Nabla to generate a overload that hits a rule that returns
nothing.