Skip to content

Conversation

@ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Oct 22, 2025

This could cause spurious disconnected errors, because the tensorified variable was not in the graph of the cost


📚 Documentation preview 📚: https://pytensor--1685.org.readthedocs.build/en/1685/

@ricardoV94 ricardoV94 added bug Something isn't working gradients labels Oct 22, 2025
Copy link
Member

@jessegrabowski jessegrabowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but mypy is failing.

How did this come up?

@ricardoV94
Copy link
Member Author

How did this come up?

I was trying to use it in the loop autodiff example. Ended up not wanting it but fixed it anyway

ricardoV94 and others added 2 commits January 7, 2026 21:29
This could cause spurious disconnected errors, because the tensorified variable was not in the graph of the cost
@jessegrabowski jessegrabowski force-pushed the gradient_of_scalars_bug branch from 4fa414c to 42f1367 Compare January 8, 2026 03:31
@jessegrabowski
Copy link
Member

This should be fine now, lmk if you object to the mypy fix. The same thing came up in #1828 . If we use Sequence in the type hint, we shouldn't check for list | tuple in the code. I changed the runtime check but we could also change the typehints.

_f: list[Variable] = [pytensor.tensor.as_tensor_variable(f)]
else:
_f = [pytensor.tensor.as_tensor_variable(x) for x in f]
if not isinstance(eval_points, Sequence):
Copy link
Member Author

@ricardoV94 ricardoV94 Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annoying thing is that Sequence includes str, so it's not always the thing we want. Fine here

@ricardoV94 ricardoV94 merged commit b2d8bc2 into pymc-devs:main Jan 9, 2026
66 checks passed
ricardoV94 added a commit to ricardoV94/pytensor that referenced this pull request Jan 11, 2026
* Do not coerce gradients to TensorVariable

This could cause spurious disconnected errors, because the tensorified variable was not in the graph of the cost

* Type-consistent checks

---------

Co-authored-by: jessegrabowski <jessegrabowski@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working gradients

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants