JuliaDiff / DiffRules.jl

A simple shared suite of common derivative definitions

Other

75 stars 38 forks source link

Avoid breaking when x is negative #36

Closed thebhatman closed 3 years ago

thebhatman commented 5 years ago

This PR is regarding this issue : https://github.com/FluxML/Zygote.jl/issues/247. So basically, Zygote is trying to calculate the derivative with respect to both base and exponent, due to the DiffRule mentioned for literal_pow. But when x is negative, it breaks due to domain error on log. So I feel it's better to check for positive base, before calculating the derivative as f(x) = a ^ x is differentiable only when a > 0.

Closes #27

MikeInnes commented 5 years ago

I'm guessing this will at least need to coerce the NaN to the right type. (e.g. the type of log(x)).

There's also some question as to whether this should be NaN or 0. NaN seems like it could be annoying in practice, and possibly not available for all floating point types (?).

I'd be interested to hear @willtebbutt's opinion, if any.

arnavs commented 4 years ago

How would I go about using this PR for my own Zygote work? Running into the same issue. Is it enough to check out the right branch of DiffRules?

MikeInnes commented 4 years ago

We should probably just define this rule in Zygote itself, overriding the DiffRules one. A PR would be great if someone's up for it.

arnavs commented 4 years ago

@MikeInnes Sorry, missed your reply on this. Would it be literally the exact same line of code?

MikeInnes commented 4 years ago

More or less. You do need to adapt it to how Zygote defines adjoints, which is in the docs.

codecov-commenter commented 3 years ago

Codecov Report

Merging #36 (5af27fe) into master (0886582) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #36   +/-   ##
=======================================
  Coverage   94.07%   94.07%           
=======================================
  Files           2        2           
  Lines         135      135           
=======================================
  Hits          127      127           
  Misses          8        8

Impacted Files	Coverage Δ
src/rules.jl	`99.20% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0886582...5af27fe. Read the comment docs.