pymc-devs / pymc-devs/pytensor
Constant.signature() is broken or missing for several subclasses
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 644
- Forks
- 208
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 16
Description
Description
Constant.signature() is the contract used for value-based equality and hashing (used by merge rewrites via merge_signature(), AtomicVariable.equals(), and FrozenApply interning). Several Constant subclasses have broken or missing implementations.
Problems found
- ScalarConstant had no signature() override (fixed in #2043)
It inherited the base Constant.signature() which returns (self.type, self.data). For NaN values, this breaks after pickle round-trip because nan != nan, causing C
module cache key lookup failures.
- TypedListConstant.signature() crashes
The base Constant.signature() returns (self.type, self.data) where data is a Python list of arrays. Both __eq__ and __hash__ are broken:
>>> c1.signature() == c2.signature()
# ValueError: The truth value of an array with more than one element is ambiguous.
>>> hash(c1.signature())
# TypeError: unhashable type: 'list'
- Bare Constant() instantiations bypass subclass signatures
Several places create Constant(type, data) directly instead of the appropriate subclass:
- Constant(slicetype, slice(None)) in type_other.py (should be SliceConstant)
- Constant(generic, None) in sparse/basic.py
- Constant(node.params_type, params) in link/c/basic.py
This is a bigger problem of:
- Variables can bypass proper subclass constructors
As noted in pytensor/scalar/basic.py L1000-1001:
# NOTE: We may have a Variable with ScalarType that is not ScalarVariable/ScalarConstant/ScalarSharedVariable
# if it bypasses those constructs (same issue with TensorVariables). We should make that impossible to happen!
A Constant with ScalarType that isn't a ScalarConstant would use the broken base signature.
We e shouldn't ever create Variable(type=ScalarType|TensorType|...) that is not a ScalarVariable|TensorVariable. Code may handle it wrongly depending on whether it checks for isinstance(x, TensorVariable) or isinstance(x.type, TensorType).
Potential solutions
- Add proper signature() to TypedListConstant
- Replace bare Constant() calls with appropriate subclasses
- Consider enforcing Type.constant_type at construction time so Constant(some_type, data) dispatches to the right subclass (factory new)
- Consider making Constant.signature() abstract (currently blocked by bare Constant() usages)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with Constant.signature() and its uses in merge_signature(), AtomicVariable.equals(), and FrozenApply interning. Inspect the bare Constant constructions in type_other.py, sparse/basic.py, and link/c/basic.py, along with the constructor concern at pytensor/scalar/basic.py lines 1000-1001. Done means affected Constant subclasses have reliable equality and hashing without the reported crashes or round-trip failures.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 42/100