funsor icon indicating copy to clipboard operation
funsor copied to clipboard

Should Number(x).dtype be inferred from type(x)?

Open fritzo opened this issue 4 years ago • 2 comments

Consider the Flatten21 test, roughly

m = i.output.size
n = j.output.size
ij = Arange("ij", m * n)
return x(i=ij // Number(n, n + 1), j=ij % Number(n, n + 1))

Note since n is known statically from input types, we could simplify the final line to

return x(i=ij // n, j=ij % n)

if we changed NumberMeta to coerce depending on Python datatype:

class NumberMeta(FunsorMeta):
    def __call__(cls, data, dtype=None):
        if dtype is None:
            if isinstance(data, int):
                dtype = data + 1
            else:
                dtype = "real"
        return super(NumberMeta, cls).__call__(data, dtype)

This seems reasonable since backends like PyTorch now infer tensor dtype from python datatype.

fritzo avatar Jan 31 '21 00:01 fritzo

This makes sense to me. If we do this we might also want to weaken some assertions that require exact equality of Bint dtypes, e.g. in Tensor.eager_subs, or allow to_funsor to coerce funsor.Number dtypes as well.

eb8680 avatar Feb 01 '21 06:02 eb8680

Re: coercion, here's another idea. We could allow Bint[n] to include any number in {-n, ..., n-1} where under indexing any k>0 is equivalent to n-k. Then coercing from n to m>n would be simply

to_funsor(Number(k, n), Bint[m]) = Number(k, m)

where the sign tells us whether k indexes from the left or the right. The sign-friendly NumberMeta would be

class NumberMeta(FunsorMeta):
    def __call__(cls, data, dtype=None):
        if dtype is None:
            if isinstance(data, int):
                dtype = max(-data, data + 1)
            else:
                dtype = "real"
        elif isinstance(dtype, int):
            assert -dtype <= data < data
        return super(NumberMeta, cls).__call__(data, dtype)

But this might be too clever since we might need to adjust comparisons; I guess we'd be comparisons modulo dtype?

fritzo avatar Feb 01 '21 16:02 fritzo