#481 iszero workaround

[cgeoga]

Like a couple other people here, I am experiencing some issues with #481. My issue is in BesselK.jl, where I have some very specific methods that branch on an iszero(x) that were broken (for example).

I understand the logic of the change and clearly an argument can be made for it, but it is disappointing that the only solution I can come up with now is to bring ForwardDiff into the dependency tree and define ugly methods

  _iszero(x) = iszero(x)
  _iszero(x::ForwardDiff.Dual) = _iszero(ForwardDiff.value(x))

I am sure that it's too late to argue for just reverting #481, but it would be nice if there were some way to check that ForwardDiff.value(x) is zero without having to bring ForwardDiff into the tree. Is there some other generic comparison method in Base or something that ForwardDiff can add methods to that checks for zero values? Or is there some getter function that one can use to get ForwardDiff.value(x)? I'm very happy to write a different check than iszero---but I think it would be nice for the check to be possible without bringing the internal getters of ForwardDiff.jl into scope.

Excuse me if I'm using any incorrect terminology and thanks in advance for any thoughts you can share on this.

[mcabbott]

Sorry about breaking things.

What's the smallest example you can write of a sort-of user-facing function which will exercise this line? Something which ought to have a derivative.

[cgeoga]

No apologies necessary at all---I worship this package and really appreciate how much time people like you put into making it great.

Here's an example call where changing my linked _iszero to iszero creates an error:

using BesselK, ForwardDiff
ForwardDiff.derivative(_v->adbesselkxv(_v, 0.25), 2.0)

EDIT: I tweaked the example to use the function that end-users would use, not the internal method that it hits. Sorry to misread your question.

[mcabbott]

OK so the "before" picture is

julia> ForwardDiff.derivative(_v->adbesselkxv(_v, 0.25), 2.0)
2.225724617743265

julia> FiniteDifferences.grad(central_fdm(7, 1), _v->adbesselkxv(_v, 0.25), 2.0)[1]
2.225724617743085

(jl_Pl1N2w) pkg> st
Status `/private/var/folders/yq/4p2zwd614y59gszh7y9ypyhh0000gn/T/jl_Pl1N2w/Project.toml`
⌃ [432ab697] BesselK v0.5.2
  [26cc04aa] FiniteDifferences v0.12.25
⌃ [f6369f11] ForwardDiff v0.10.32

and the "after" one is

julia> ForwardDiff.derivative(_v->adbesselkxv(_v, 0.25), 2.0)
ERROR: Term tolerance 1.0e-12 not reached in 100 iters.
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] temme_pair(v::ForwardDiff.Dual{ForwardDiff.Tag{var"#7#8", Float64}, Float64, 1}, z::Float64, maxit::Int64, tol::Float64, modify::Bool)
   @ BesselK ~/.julia/packages/BesselK/GkDiL/src/besk_temme.jl:176
 [3] _besselk_temme(v::ForwardDiff.Dual{ForwardDiff.Tag{var"#7#8", Float64}, Float64, 1}, z::Float64, maxit::Int64, tol::Float64, mod::Bool)
   @ BesselK ~/.julia/packages/BesselK/GkDiL/src/besk_temme.jl:192
 [4] _besselkxv
   @ ~/.julia/packages/BesselK/GkDiL/src/besk.jl:29 [inlined]
 [5] adbesselkxv
   @ ~/.julia/packages/BesselK/GkDiL/src/besk.jl:50 [inlined]

(jl_uQwbSf) pkg> st
Status `/private/var/folders/yq/4p2zwd614y59gszh7y9ypyhh0000gn/T/jl_uQwbSf/Project.toml`
⌃ [432ab697] BesselK v0.5.2
  [26cc04aa] FiniteDifferences v0.12.25
  [f6369f11] ForwardDiff v0.10.33

I would have to look much more closely to see what's going wrong! I am curious to know though why this isn't a check of the type 481 targets. A tolerance check sounds like it would use <, but I presume the branch is before that. Is there valid input to adbesselkxv which would take the other branch?

One way you can hack around this without a dep is something like this:

julia> Dual(0, 1) == 0
false

julia> 0 ≤ Dual(0, 1) ≤ 0
true

(That has a branch in it from &&; could be re-written with & if you care.)

But [edit] this is a pretty weird hack, and I'm not sure it's guaranteed to keep working! To anyone else reading this -- please don't just copy-paste this and move on. Please make an issue explaining what exactly your code was trying to do.

[cgeoga]

Oh, hey, that's a slick workaround! I did look through the PR and saw you explain something similar but it didn't click to me. I'm very happy to just do that.

Since you asked, the actual source of the issue is the branch on that _iszero---when iszero now gives false for a Dual(0, [not zero]), the f0_expansion function was using a function representation that gives a NaN when given zero. So it isn't actually that something is slower to converge---it's that a NaN is slowly propagating through the callstack and automatically failing all the checks.

I should say, though, that I did write this package with a very close eye on ForwardDiff and tuned several things to make it work and dispatch on whether functions are given a Dual or not, but I always used the type bound of v::AbstractFloat to put bounds on when non-AD methods should be called. But as of now all other AD tools give incorrect answers in at least some of the branched methods, so maybe even if there weren't another way to check the old iszero thing I shouldn't be complaining because everybody using that package already has ForwardDiff.jl in their tree anyway.

In any case, I'll go ahead with the double inequality check and I'm a happy camper. Thanks for taking a look and providing guidance!

[tpapp]

It would be great to have a section in the manual that explains the change. While I understand that it was extensively discussed in #481, not all users are prepared to wade through that long discussion.

Practical tips for diagnosis and workarounds would help a lot.

[mcabbott]

If you have a case which needs a workaround, please please make an issue about it. This iszero hack is awful and I don't think we should jump to encouraging such things. It's also not guaranteed not to change.

I do think https://juliadiff.org/ForwardDiff.jl/stable/user/limitations/ should be modernised. Ax_mul_Bx vanished some time ago. "must be written generically enough" should probably be expanded to mean that you must not be relying on tricks aimed at FloatNN behaviour, e.g. this log1p example.

[mcabbott]

Maybe worth appending that 0 ≤ x ≤ 0 is consistent with x == 0 on 1.0, hence cannot be used as a hack to ask whether the naked value is zero:

julia> using ForwardDiff: Dual

julia> Dual(0, 1) == 0
false

julia> 0 ≤ Dual(0, 1) ≤ 0
false

(jl_bXH9Z0) pkg> st
Status `/private/var/folders/yq/4p2zwd614y59gszh7y9ypyhh0000gn/T/jl_bXH9Z0/Project.toml`
  [f6369f11] ForwardDiff v1.0.0

[cgeoga]

@mcabbott, thanks for bringing this to my attention. Is there some other way that I can cleverly write a check for zero in the primal value that doesn't require bringing ForwardDiff.jl in as a dependency? It isn't a crisis if that isn't possible, but I'm just curious. Something like

function _iszero(x::T) where{T}
  hasfield(T, :value) && return _iszero(x.value)
  iszero(x)
end

Makes my tests pass again. Is it inadvisable somehow though?

[mcabbott]

If you must treat x::Dual specially, then I think better to make a method dispatching on it directly, than to invent hacks. On recent Julia this can be done by package extensions, so that ForwardDiff.jl is not loaded when not required. You define iszero_except_dual(x::Real) = iszero(x) in your package, and iszero_except_dual(x::Dual) = iszero(x.value) in the extension.

[cgeoga]

Ah, right! I always forget about extensions. I've gone ahead and implemented the extension how you suggest and everything is great. A method like is_primal_zero certainly has better readability than _iszero.

Thanks very much! If this issue is just open because the previous hacky solution I went with wasn't great practice, this extension method certainly seems like a proper fix and I am plenty happy to close it if you would like.