pymc-devs / pytensor

PyTensor allows you to define, optimize, and efficiently evaluate mathematical expressions involving multi-dimensional arrays.
https://pytensor.readthedocs.io
Other
371 stars 109 forks source link

Default casting to int8 leads to easy overflow errors #1073

Open daniel-s-tccc opened 3 weeks ago

daniel-s-tccc commented 3 weeks ago

Describe the issue:

Minimal example below but if you pass an 'int8' to the gamma distribution, you cannot draw from it. If you make it int16, however, no problem. This problem can show up unexpectedly when a chain of pytensor operations downcasts.

It is also sensitive to the value of mu. If mu is 51, it doesn't happen.

Reproduceable code example:

import pymc as pm
import pytensor.tensor as pt

mu = pt.as_tensor(50,dtype="int8")

rv = pm.Gamma.dist(
    mu = mu,
    sigma = 1
)
pm.draw(rv)

Error message:

ValueError: shape < 0
Apply node that caused the error: gamma_rv{0, (0, 0), floatX, True}(RandomGeneratorSharedVariable(<Generator(PCG64) at 0x1C107CE2260>), [], 11, -60.0, 0.02)
Toposort index: 0
Inputs types: [RandomGeneratorType, TensorType(int64, shape=(0,)), TensorType(int64, shape=()), TensorType(float64, shape=()), TensorType(float64, shape=())]
Inputs shapes: ['No shapes', (0,), (), (), ()]
Inputs strides: ['No strides', (0,), (), (), ()]
Inputs values: [Generator(PCG64) at 0x1C107CE2260, array([], dtype=int64), array(11, dtype=int64), array(-60.), array(0.02)]
Outputs clients: [['output'], ['output']]

PyMC version information:

pymc: 5.14.0 pytensor: 2.20.0

Context for the issue:

No response

welcome[bot] commented 3 weeks ago

Welcome Banner] :tada: Welcome to PyMC! :tada: We're really excited to have your input into the project! :sparkling_heart:
If you haven't done so already, please make sure you check out our Contributing Guidelines and Code of Conduct.

ricardoV94 commented 3 weeks ago

Love the conciseness of the reproducible example :)

ricardoV94 commented 3 weeks ago

You're seeing overflow. The 51value is still wrong, just overflowing to a positive value.

import pytensor.tensor as pt

(pt.as_tensor([50, 51], dtype="int8") ** 2).eval()  # array([-60,  41], dtype=int8)
np.array([50, 51], dtype="int8") ** 2  # array([-60,  41], dtype=int8)
ricardoV94 commented 3 weeks ago

The difference between pytensor and numpy is the default type of as_tensor:

np.array(50).dtype  #  dtype('int64')
pt.as_tensor(50).dtype  # 'int8'
ricardoV94 commented 3 weeks ago

Maybe it's time we change the default casting policy?

import pytensor
import pytensor.tensor as pt

with pytensor.config.change_flags(cast_policy="numpy+floatX"):  # instead of default `"custom"`
    x = pt.as_tensor(50)
x.dtype  # 'int64'    
daniel-saunders-phil commented 3 weeks ago

Seems reasonable to me, might reduce the number of surprises users get if we copy the numpy casting policy. I'm going make the change, runs the tests and see if anything breaks. There is this little note in the casting policy which is a bit foreboding.

# The 'numpy' policy was originally planned to provide a
# smooth transition from numpy. It was meant to behave the
# same as numpy+floatX, but keeping float64 when numpy
# would. However the current implementation of some cast
# mechanisms makes it a bit more complex to add than what
# was expected, so it is currently not available.
# numpy

If a total change of the policy isn't possible, where might I look for the rules that the default for integers only so that this could be pt.as_tensor(50).dtype # 'int64'?

ricardoV94 commented 3 weeks ago

numpy+floatX is probably a better default for us anyway, so I would try that