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. On
onnxruntime 1.23.2 for macOS arm64 the zero divisor raises nothing and returns the
dividend, which Slice clamps back into range so the result still looks correct; on the build in
#3024 it fails with Integer modulo by zero.

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 and left the behavior exactly as it is, so the graph is unchanged from main. The limitation is now written at both helpers next to the asserts from #3024: the check only covers a length known to be zero at export time, a dynamic dimension that is zero at run time still divides by zero there, and onnxruntime throws on some builds and quietly returns the dividend on others. I will look at the onnxruntime paths separately, though I would rather not put a date on it.