Skip to content
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 rrule for infinite-norm #204

Closed
sethaxen opened this issue May 27, 2020 · 0 comments · Fixed by #226
Closed

Add rrule for infinite-norm #204

sethaxen opened this issue May 27, 2020 · 0 comments · Fixed by #226

Comments

@sethaxen
Copy link
Member

The current rrule for norm fails when p=Inf, for which the norm is equivalent to maximum(abs, x):

julia> using LinearAlgebra

julia> x = Float64[1, 2, -3];

julia> norm(x, Inf)
3.0

julia> _, back = ChainRules.rrule(norm, x, Inf);

julia> ∂self, ∂x, ∂p = back(1.0);

julia> ∂x() # expected: [0.0, 0.0, -1.0]
3-element Array{Float64,1}:
   0.0
 NaN
 NaN

julia> ∂p() # expected: 0.0 or Zero()
NaN

This really just fails for any very large p (e.g. also fails for p=1e3). It works fine for p=100:

julia> norm(x, 1e2)
3.0

julia> _, back = ChainRules.rrule(norm, x, 1e2);

julia> ∂self, ∂x, ∂p = back(1.0);

julia> ∂x()
3-element Array{Float64,1}:
  5.820975652447899e-48
  3.689481639869744e-18
 -1.0

julia> ∂p()
0.0

The actual implementation of norm in LinearAlgebra special-cases p of 0, 1, 2, -Inf, and Inf, falling back to normp if another p is provided (see https://github.com/JuliaLang/julia/blob/master/stdlib/LinearAlgebra/src/generic.jl#L604-L619). Should the rrule for norm here instead be replaced by rrules for these functions? I see 2 downsides: 1) more code 2) pullback of p for these special p's would be Zero(), and in general would not be type-stable when pulling back through norm.
On the upside, the pullbacks for these types can be implemented a little more efficiently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants