Document that roll leaves a dynamic zero length dimension undefined

[om singhal (Om-singhaI)]

torch.roll exported with a dimension that can be zero still reaches Mod with a
zero divisor when that dimension is zero at run time. The 0 in self.shape early
return added in #3024 only covers a length known to be zero at export time, and ONNX
leaves modulo by zero undefined.

Undefined does not always mean loud, which is why it is worth writing down. The
onnxruntime CPU kernel has rejected an integer divisor of zero since microsoft/onnxruntime#27833
shipped in 1.25.0, but its CUDA kernel has no such check, and on 1.23.2 the zero divisor
raised nothing and returned the dividend, which Slice clamped back into range so the
result still looked correct.

This is a comment only change. The behavior is deliberately unchanged and the two
asserts stay as they are; each now carries the limitation next to it, and the early
return in aten_roll no longer reads as though it covers the dynamic case.

[Justin Chu (justinchuby)]

Hopefully this doesn't over complicate the implementation? It is also possible to assume input will never be zero sized, I hope?

[om singhal (Om-singhaI)]

It is small: one Max on the divisor in each of the two helpers, and it let me drop the two asserts that used to claim a zero could never reach them, so the net is close to even.

On assuming nonzero, I would rather not. torch.roll on an empty tensor is legal and returns it unchanged, and a dim declared min=0 is a shape the exporter was told to expect. What makes it awkward is that Mod by zero is undefined rather than just unusual, so the same model throws on your build and quietly returns a clamped result on mine.

It is a rare shape though. If you would rather not carry the guard at all I am happy to close this. One middle option if the cost is what bothers you: only emit the Max when the length is not a static int, since a static one has already returned Identity for a zero upstream. Every static export then comes out byte identical to today and the guard appears only where it cannot be proven unnecessary.

[Justin Chu (justinchuby)]

Thanks. I suggest we note the limitation clearly in the source, and then keep the status quo until someone actually needs the behavior.

For onnxruntime, it would be great if you can update the different paths to all produce an error.

[om singhal (Om-singhaI)]

Dropped the Max guard, so the graph is identical to main. The limitation is written at both helpers now, next to the asserts from #3024: the check only catches a length known to be zero at export time, a dynamic dimension that's zero at run time still divides by zero, and runtimes don't agree on what that does.

I'll look at the onnxruntime paths separately, though I'd rather not put a date on it.

[om singhal (Om-singhaI)]

Correcting myself. The CPU kernel already errors on an integer divisor of zero, microsoft/onnxruntime#27833 added that and it shipped in 1.25.0. I'm on 1.23.2, so what I measured was a version gap, not a platform one. I've reworded the note in both helpers.

CUDA is still unguarded though. _Mod in common.cuh is a % b plus a sign fix, no zero check.

I can't build onnxruntime to fix that, no NVIDIA hardware, and their guidelines want a build and a test run first. Happy to file an issue with the file and line if that's useful.