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

LogSumExp of -inf returns nan instead of -inf #461

Closed
ricardoV94 opened this issue Jun 4, 2021 · 1 comment · Fixed by #465
Closed

LogSumExp of -inf returns nan instead of -inf #461

ricardoV94 opened this issue Jun 4, 2021 · 1 comment · Fixed by #465
Labels
bug Something isn't working graph rewriting help wanted Extra attention is needed important

Comments

@ricardoV94
Copy link
Contributor

ricardoV94 commented Jun 4, 2021

x = at.vector('x')
res1 = at.log(at.sum(at.exp(x)))  # nan with sum
res2 = at.log(at.prod(at.exp(x)))  # but not with prod

fun = aesara.function([x], [res1, res2])
print(fun(np.array([-np.inf, -np.inf])))  # [array(nan), array(-inf)]

for reference in numpy we get -inf, together with a divide by zero encountered in log

print(np.log(np.sum(np.exp([-np.inf, -np.inf]))))  # -inf

This showed up in the pymc3 logsumexp function which returns nan with this input:

from pymc3.math import logsumexp

x = at.vector('x')
res = logsumexp(x)
fun = aesara.function([x], res) 

print(fun(np.array([-np.inf, -np.inf])))  # [nan]

Whereas the scipy reference works fine

print(scipy.special.logsumexp([-np.inf, -np.inf]))  # -inf

Weirdly it happens with addition, but not subtraction or multiplication, so maybe the problem is not the sum but the addition?

x = at.vector('x')
res1 = at.log(at.exp(x[0]) + at.exp(x[1]))
res2 = at.log(at.exp(x[0]) - at.exp(x[1]))
fun = aesara.function([x], [res1, res2])
print(fun(np.array([-np.inf, -np.inf])))  # [array(nan), array(-inf)]
@ricardoV94 ricardoV94 changed the title Log of Sum of Zero returns nan instead of -inf Log of Sum of zero returns nan instead of -inf Jun 4, 2021
@ricardoV94 ricardoV94 changed the title Log of Sum of zero returns nan instead of -inf LogSumExp of -inf returns nan instead of -inf Jun 4, 2021
@ricardoV94
Copy link
Contributor Author

ricardoV94 commented Jun 4, 2021

I think it has to do with an automatic optimization which is basically the same as what the pymc3 logsumexp attempts to do, but with this line missing:

https://github.com/pymc-devs/pymc3/blob/d7172c0a1a76301031d1b3b411d00643c416a0c4/pymc3/math.py#L192

x = at.vector('x')
res = at.log(at.exp(x[0]) + at.exp(x[1]))
fun = aesara.function([x], res)
aesara.dprint(fun)
Elemwise{Composite{(maximum(i0, i1) + scalar_softplus(((i0 - maximum(i0, i1)) + (i1 - maximum(i0, i1)))))}} [id A] ''   2
 |Subtensor{int64} [id B] ''   1
 | |x [id C]
 | |ScalarConstant{0} [id D]
 |Subtensor{int64} [id E] ''   0
   |x [id C]
   |ScalarConstant{1} [id F]

The softplus input in the graph above has a -inf - (-inf), yielding the nan in the output

@brandonwillard brandonwillard added bug Something isn't working important graph rewriting help wanted Extra attention is needed labels Jun 4, 2021
ricardoV94 added a commit to ricardoV94/aesara that referenced this issue Jun 6, 2021
ricardoV94 added a commit to ricardoV94/aesara that referenced this issue Jun 7, 2021
ricardoV94 added a commit to ricardoV94/aesara that referenced this issue Jun 7, 2021
ricardoV94 added a commit to ricardoV94/aesara that referenced this issue Jun 7, 2021
ricardoV94 added a commit to ricardoV94/aesara that referenced this issue Jun 7, 2021
ricardoV94 added a commit to ricardoV94/aesara that referenced this issue Jun 7, 2021
ricardoV94 added a commit to ricardoV94/aesara that referenced this issue Jun 9, 2021
ricardoV94 added a commit to ricardoV94/aesara that referenced this issue Jun 9, 2021
ricardoV94 added a commit that referenced this issue Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working graph rewriting help wanted Extra attention is needed important
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants