Change `af::seq` representation to allow empty sequence
Issue
Currently af::seq(0) incorrectly produces a sequence representing "all elements along an axis", instead of an empty sequence.
Demo:
auto x = af::constant(3, 3);
std::cout << af::toString("", x(af::seq(0))) << std::endl; // prints array of 3 elements
In fact, there seems to be no way to construct an "empty sequence" in ArrayFire at the moment.
Proposal
Currently in af::seq, a step size of 0 is used to represent span. Also see the static span instance.
However, we could naturally represent a span via af::seq(0, -1, 1).
By doing that, we can now use step size of 0 to represent empty sequence (which would otherwise be difficult due to the inclusive semantics of af::seq::end. We also need to figure out semantics/constraints of start and end in that case, but it should be pretty flexible.
Benefits
- It makes
af::seq's semantics more complete, i.e., makesaf::seq(0)correct. - It may help clients of ArrayFire, e.g., Flashlight. cc @jacobkahn
If this is causing a breaking change, I would instead suggest changing af::seq to have an exclusive end, which is the original problem. That is the way standard C++ iterators and ranges work, as well as Python ranges, so there is a good precedent in favor of that. But beyond the "others are doing it" argument, there are objective advantages.
It allows handling empty sequences naturally in generic code without needing explicit checks. For example, it would make x(af::seq(begin, end)) valid for any begin <= end and end <= x.elems(), no matter if x is empty or not and no matter what the step is. With the change proposed by @StrongerXi, the special case of x being empty needs to be handled explicitly by setting the step to zero, or wrapping the whole calculation in an if statement to bypass it.
Since ArrayFire uses zero-based indexing, it also makes it more natural to write a range that includes the end, such as x(af::seq(x.elems() - 2, x.elems()). This is not as big a deal as in other libraries/languages, since ArrayFire has negative indexing to handle this case as well, currently as x(af::seq(-2, -1)). ~~However ArrayFire doesn't seem to allow mixing positive and negative bounds in the same af::seq() object, so if you need a range that excludes the first and last element, you currently need to write af::seq(1, x.elems() - 2), which is somewhat confusing.~~ Edit: I'm wrong, misread the code :) The argument still stands if people are reluctant to use negative indices for any (psychological) reason.
I agree. I didn't propose the inclusive -> exclusive change because I thought it's a bit too aggressive:). But yes, I do think it's more natural (based on my own experiences with various "range" representations in different languages/libraries).
One thing to note is that, with exclusive end, we can't replicate this easily: af::seq(0, af::end), because we don't have a candidate in the integer/double space to represent af::end (the equivalent in the exclusive end world is "one index beyond the last index in an axis"). We could make it optional<double> optional though, which is what Python does if you write x[0:].
tl;dr:
- I agree exclusive end feels more natural.
- Exclusive end may introduce complexity for
af::seq(0, af::end).