Does the fix affect the overload torch.arange(end)? The corresponding ATen code seems to call in the legacy function without the new check: https://github.com/pytorch/pytorch/blob/f472a5075c83b26a5f1177b214ce3f0d691197c3/aten/src/ATen/native/TensorFactories.cpp#L98

The same for all arange_out overloads

Does the fix affect the overload torch.arange(end)?

nope, that's still broken. I'l fix it that too and add tests for other cases.

@nairbv I don't understand PyTorch's internals very well. Is _th_arange_out also handled by the (now fixed) THTensor_(arange)(THTensor *r_, accreal xmin, accreal xmax, accreal step)? (and there isn't a test case for this, so I am not sure)

Appears so:

Tensor & CPUFloatType::_th_arange_out(Tensor & result, Scalar start, Scalar end, Scalar step) const {
    // DeviceGuard omitted
    auto result_ = checked_tensor_unwrap(result,"result",0, false, Backend::CPU, ScalarType::Float);
    auto start_ = start.toDouble();
    auto end_ = end.toDouble();
    auto step_ = step.toDouble();
    THFloatTensor_arange(result_, start_, end_, step_);
    return result;
}

@nairbv I don't understand PyTorch's internals very well. Is _th_arange_out also handled by (the now fixed) THTensor_(arange)(THTensor *r_, accreal xmin, accreal xmax, accreal step)?

yes, I'm new to this as well but the easiest way to see it is if you build the code, it'll appear in the generated file CPUFloatType.cpp (generated by aten/src/ATen/gen.py).

All of the _th_arange* functions call THFLoatTensor_arange(result_, start_, end_, step_); where sometimes start_ defaults to 0 and sometimes step defaults to 1... where sometimes result_ is a parameter and sometimes result is created. macro expansions lead from the THTensor_(arange) changed here to the THFloatTEnsor_arange being called.

Is CUDA version also fixed by the edits in THTensor_(arange)?

For before it was, so CUDA was affected too:

>>> a = torch.arange(0, float('inf'), device = 'cuda')
>>> a.shape
torch.Size([-9223372036854775808])

If you move down the guard to TH (as it appears now), it probably should also be duplicated in THC:

Can an overflow result in a positive value or even a zero? Probably not? (depends on how many times the overflow rolls - probably not more than 1?)

Can an overflow result in a positive value or even a zero? Probably not? (depends on how many times the overflow rolls - probably not more than 1?)

hm... it seems that the behavior we're seeing is in the cast from double to int64_t (or actually, to ptrdiff_t). From what I can tell the behavior for an out of range cast is undefined, but in practice it doesn't seem to be wrapping. You saw this -9223372036854775808 value in tests (the minimum long value, or max long + 1).

We could try a range check on the double first. That's tricky though because ptrdiff_t's max value isn't exactly representable in floating point, and could differ in 32 bit systems. I'm not sure if there is actually a system that would wrap it, but I'm also pretty new to C++.

I'm a bit skeptical of the floating point arithmetic in general of (max-min)/step, but I don't see a specific issue with it yet, and we might be getting beyond the scope of the original ticket. Maybe we should get this merged, and file another ticket if we find another specific failure case?

One of the newly-added tests did end up failing in one environment with:

Nov 19 23:30:46 test_arange (main.TestTorch) ... /var/lib/jenkins/workspace/aten/src/TH/generic/THTensorMoreMath.cpp:645:22: runtime error: 3.40282e+38 is outside the range of representable values of type 'long'

so I guess we will need to explicitly check the ranges before casting.

@pytorchbot retest this please

I'm a bit skeptical of the floating point arithmetic in general of (max-min)/step, but I don't see a specific issue with it yet, and we might be getting beyond the scope of the original ticket. Maybe we should get this merged, and file another ticket if we find another specific failure case?

I don't think anyone is actually blocked by the lack of a check here. Let's take the time to do it right.