💊 CI failures summary and remediations

As of commit ea11981 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

A lot of real-world usage cases can be found in https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/Loss.cpp

It would be good if they were refactored to not allocate zeros tensors.

Another question is that they often allocate indicator tensors (often it's like x == 1, x == -1, x > 0, x == 0), but without automatic fusion or introducing comparison op argument indicating on how to interpret a float tensor (or choosing one by default, e.g. 0 is 0, non-zero is 1, like in C), it's impossible to get rid of and maybe not worth it - also discussed in #9190

Will type promotion be also be supported, i.e. using 0 or 1 in place of 0.0 and 1.0?

@vadimkantorov

A lot of real-world usage cases can be found in https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/Loss.cpp
It would be good if they were refactored to not allocate zeros tensors.

Thanks! Would be a good test as well.

Another question is that they often allocate indicator tensors (often it's like x == 1, x == -1, x > 0, x == 0), but without automatic fusion or introducing comparison op argument indicating on how to interpret a float tensor (or choosing one by default, e.g. 0 is 0, non-zero is 1, like in C), it's impossible to get rid of and maybe not worth it - also discussed in #9190

Sounds interesting but surely out-of-scope for this PR.

Will type promotion be also be supported, i.e. using 0 or 1 in place of 0.0 and 1.0?

I am not planning to do it in this PR as it would also affect the behaviour of Tensor-Tensor overload.

I am not planning to do it in this PR as it would also affect the behaviour of Tensor-Tensor overload.

Got it. for future, type promotion would be useful for tensor-tensor overload too (although, not as pressing, given the new scalar support)

Also, not sure if torch.lerp currently supports scalars or not

Also, at some point there were problems of torch.where producing NaN gradients, it would be good to have a note in docs about current state of affairs (canonical erroring example is entropy computation in presence of 0 torch.where(p > 0, p*p.log(), 0).sum(-1)):
#18287 #23395

Many of these issues were circularly closed (one links another), the only currently open issue is: #23156

@mruberry Please review :)

Will sth like

x = torch.rand(3, 4, 5).to('cuda')
y = torch.where(x > 0.5, x, torch.tensor(0))

work?
i.e. cpu->cuda scalar copy

This PR doesn't actually touch the current version.

>>> a = torch.randn(3,4).to('cuda')
>>> torch.where(a > 0.5, a, torch.tensor(0.))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: Expected condition, x and y to be on the same device, but condition is on cuda:0 and x and y are on cuda:0 and cpu respectively

However after this PR

>>> a = torch.randn(3,4).to('cuda')
>>> torch.where(a > 0.5, a, 0)
tensor([[0.0000, 0.0000, 0.5484, 0.0000],
        [0.8243, 0.0000, 1.0878, 1.2405],
        [0.0000, 0.0000, 0.0000, 1.7502]], device='cuda:0')

@mruberry
Gentle ping :)

@mruberry

Have address the changes.
Do let me know if there is a better way to phrase the comment in ScalarOps.h.

For next iteration (type-promotion), we would most likely have tests directly against numpy!

@mruberry Gentle Ping:)

@mruberry

Have address the changes.
Do let me know if there is a better way to phrase the comment in ScalarOps.h.

For next iteration (type-promotion), we would most likely have tests directly against numpy!

Sorry to keep you waiting, @kshitij12345, it's been a very busy week! Thank you for your patience.

Let's not hold this PR up anymore on small issues. We can look at improving that comment, if we want to, in the future.

Internal tests flagged a few potential issues / lint problems, @kshitij12345. I'll sort through them tomorrow. Should be a few simple fixes.

@mruberry
Sure. Thank You!
Do let me know if I can help with anything.

@mruberry merged this pull request in 31d41f9.

Also, at some point there were problems of torch.where producing NaN gradients, it would be good to have a note in docs about current state of affairs (canonical erroring example is entropy computation in presence of 0 torch.where(p > 0, p*p.log(), 0).sum(-1)):
#18287 #23395

Many of these issues were circularly closed (one links another), the only currently open issue is: #23156

Does the entropy case work now? Is torch.where NaN-safe for this case?

Does the entropy case work now? Is torch.where NaN-safe for this case?

No. The behavior of torch.where is unchanged. This is just sugar allowing scalars to be interpreted as tensor arguments.

Does the entropy case work now? Is torch.where NaN-safe for this case?

No. The behavior of torch.where is unchanged. This is just sugar allowing scalars to be interpreted as tensor arguments.

It would be good to have this as a test, even if it's known to fail at the moment. A note in docs would also be good. This is a very common mistake (and entropy computation usecase specifically is a good/common example of this I think) and source of surprises...

import torch

def where_is_not_nan_safe():
  p = torch.zeros(4).requires_grad_()
  e = torch.where(p > 0, p*p.log(), torch.zeros_like(p)).sum(-1)
  assert torch.isnan(torch.autograd.grad(e, (p,))[0]).all()

The linked issue is less about torch.where and more about how gradients are represented, so I don't think we'd take a test for this in particular.

You're right. From current gradient processing + no special processing in torch.where, it's how things are. But if torch.where does special processing, this could be a way around current gradient processing. All I wanted to point out is that trying to implement entropy and work around the naive p * p.log() with torch.where is quite frequent (and failing currently), to the point that it might deserve a note in docs, and at best be fixed by special processing in torch.where (if technically possible).