Fix deprecation warning introduced by DiffRules v1.4 #553
Conversation
src/dual.jl
Outdated
| ################################### | ||
|
|
||
| for (M, f, arity) in DiffRules.diffrules() | ||
| for (M, f, arity) in DiffRules.diffrules(filter_modules = (:Base, :SpecialFunctions, :NaNMath)) |
There was a problem hiding this comment.
I guess one could use the opportunity and already prepare this code for future releases of DiffRules. As stated in the deprecation warning it will change to filter_modules=nothing, ie. it will load all rules defined in DiffRules. We only had to introduce this keyword argument because many downstream packages, including ForwardDiff, did not actually check if a module M with a function f was defined when they tried to load the rules.
So an alternative approach here that would also add derivative definitions for functions in LogExpFunctions would be to:
- add LogExpFunctions >= 0.3 as explicit dependency (it's already an indirect dependency via DiffRules and SpecialFunctions)
- use
DiffRules.diffrules(; filter_modules=nothing)and load all rules, including the ones for LogExpFunctions - check if
MandM.fare defined before trying toevalthe rule definitions
| for (M, f, arity) in DiffRules.diffrules(filter_modules = nothing) | ||
| if (M, f) in ((:Base, :^), (:NaNMath, :pow), (:Base, :/), (:Base, :+), (:Base, :-)) | ||
| continue # Skip methods which we define elsewhere. | ||
| elseif !(isdefined(@__MODULE__, M) && isdefined(getfield(@__MODULE__, M), f)) |
There was a problem hiding this comment.
Is this a reasonable way to check if M and f are defined? Seems a bit clunky.
|
Why are the tests non-deterministic? Failure on 1.0 isn't caused by this: https://github.com/JuliaDiff/ForwardDiff.jl/runs/4111622597?check_suite_focus=true#step:6:293 |
|
Tests are passing now thanks to #554. |
|
Any chance we could get this merged and a new release tagged? |
|
Thanks! |
2 participants
DiffRules 1.4.0 introduced a new deprecation warning: JuliaDiff/DiffRules.jl#69
This caused the JuMP tests to fail since we run with
--depwarn=error:https://github.com/jump-dev/JuMP.jl/runs/4110519060?check_suite_focus=true#step:6:26
x-ref: jump-dev/JuMP.jl#2790