Skip to content

Conversation

mattjj
Copy link
Collaborator

@mattjj mattjj commented Feb 15, 2019

See #347 for example code.

Interestingly, Autograd still has this issue:

import autograd.numpy as np
from autograd import grad

def f(x):
  x = np.sqrt(np.sum(x**2, axis=1))
  return np.sum(np.where(x > 0.5, x, np.ones_like(x)))

x = np.array([[1, 2], [3, 4], [0, 0]], dtype=np.float64)
print grad(f)(x)

# [[0.4472136  0.89442719]
#  [0.6        0.8       ]
#  [       nan        nan]]

It comes down to the formula for the jvp of sqrt(x) being problematic at x=0 when the gradient has coefficients that are also zero, since g * x ** -0.5 is 0 * inf there. In this case, the zero coefficients in the gradient arise from the lax.select/np.where call.

I think a zero in the gradient should win out over an inf in the Jacobian, and this PR implements that policy. I had trouble implementing it on top of the standard lax.mul primitive because of linearity constraints in the JVP rule, so instead I added a new lax._safe_mul that implements the 0*inf = 0 policy in its translation rule.

@hawkinsp
Copy link
Collaborator

Travis is unhappy.

@mattjj mattjj removed the request for review from hawkinsp February 15, 2019 16:11
@mattjj
Copy link
Collaborator Author

mattjj commented Feb 15, 2019

Yeah sorry, this isn't ready for review yet!

@mattjj mattjj requested a review from hawkinsp February 16, 2019 04:26
@mattjj mattjj self-assigned this Feb 16, 2019
@mattjj mattjj merged commit 3bf5f33 into master Feb 16, 2019
@mattjj mattjj deleted the issue347 branch February 16, 2019 16:10
@mattjj mattjj restored the issue347 branch February 16, 2019 16:10
@mattjj mattjj deleted the issue347 branch February 16, 2019 16:13
@j-towns j-towns mentioned this pull request Apr 4, 2019
mattjj added a commit that referenced this pull request Mar 18, 2020
mattjj added a commit that referenced this pull request Mar 18, 2020
srvasude pushed a commit to srvasude/jax that referenced this pull request May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants