pytensor icon indicating copy to clipboard operation
pytensor copied to clipboard

Name check on `pt.tensor` is too strict

Open jessegrabowski opened this issue 1 year ago • 2 comments

Description

a0 = pt.tensor(name="a0", dtype=floatX, shape=(m,))

Fails with the following error:

    a0 = pt.tensor(name="a0", dtype=floatX, shape=(m,))
/home/jesse/mambaforge/envs/pymc-statespace/lib/python3.11/site-packages/pytensor/tensor/type.py:791: in tensor
    raise ValueError(
E   ValueError: The first and only positional argument of tensor is now `name`. Got a0.
E   This name looks like a dtype, which you should pass as a keyword argument only.

There is a check here in pt.tensor that makes sure we're not passing a dtype as the only argument to pt.vector (for historical reasons I guess?). This is fine, but it shouldn't occur if the dtype is also specified. Fixing this would be as easy as changing the if condition to if name is not None and dtype is not None, since in that case I clearly did not want a0 interpreted as a dtype.

jessegrabowski avatar Nov 22 '24 16:11 jessegrabowski

Hey @jessegrabowski I was taking a look at this one. I have a couple of theories as to why this was included as a feature, my main theory being that they wanted to let people know when they ran their code that the positional argument has changed to only have name as the single positional argument. My less likely theory is that this is a feature to try and prevent any conflicts between name and dtypes in other areas of the code? It's weird to me that they would be validating this field at all other than for UX purposes though.

philipscoderepo avatar Mar 31 '25 00:03 philipscoderepo

Yes, I believe it was for backwards compatibility with the now very, very distant past. tensor used to look like this:

def tensor(*args, **kwargs):
    name = kwargs.pop("name", None)
    return TensorType(*args, **kwargs)(name=name)

The ordering of args to TensorType is still dtype first, so the change to name as a keyword argument broke ancient code that assumed you could do something like x = pt.tensor('i1', 'x').

I think it's fine to keep the check for backwards compat purposes, but when there's no ambiguity -- for example when dtype is explicitly passed as a keyword argument -- we don't need it.

jessegrabowski avatar Mar 31 '25 04:03 jessegrabowski