Remove test_value functionality#1972
Conversation
ricardoV94
force-pushed
the
no_more_tag_test_value
branch
from
44bd9cb to
642b04c
Compare
ricardoV94
force-pushed
the
no_more_tag_test_value
branch
from
642b04c to
f35eb13
Compare
ricardoV94
force-pushed
the
no_more_tag_test_value
branch
from
f35eb13 to
086ae7b
Compare
|
A small can of worms showed up when we refactored some tests that were using tags to actually use the compilation pathway (which should be what we tested in the first place). We were asserting the following shape behavior:
Now those just fail. Shape.make_node always calls Also removed shape/ndim argument from TypedList. This isn't used anywhere and it isn't an obvious design decision. Should it be vector, none (opaque object), whatever it contains +1. Let's leave it for when we actually need it. |
ricardoV94
force-pushed
the
no_more_tag_test_value
branch
from
086ae7b to
d726ccc
Compare
Before: shape(NoneType) -> np.shape(None) -> () (numpy tends to do this for things it doesn't understand) shape(TypedLists) -> shape(np.asarray(TypedList)) -> either converts or fails. Now TypedList are considered vectors / 1d like python lists
ricardoV94
force-pushed
the
no_more_tag_test_value
branch
from
d726ccc to
2d3f6f9
Compare
|
|
||
| """ | ||
|
|
||
| dtype = property(lambda self: self.ttype) |
There was a problem hiding this comment.
Why this instead of
@property
def dtype(self) -> TypeT:
return self.ttypeThere was a problem hiding this comment.
python 2 legacy if I had to guess
There was a problem hiding this comment.
claude tells me it's idiomatic for single variable delegation and that others do the same
There was a problem hiding this comment.
If it makes you feel better, it probably prefaced it with: "That's an insightful question"
| assert np.array_equal(res.data, [1, 1]) | ||
|
|
||
| # A test for a non-`TensorType` | ||
| class MyType(Type): |
There was a problem hiding this comment.
yes please omg can we make removing all the MyType MyOp MyVar a blocker for v3
There was a problem hiding this comment.
These simple Ops and types in tests are good, they make sure we don't overcommit to tensors. But shape is a tensor Op, why should it accept arbitrary types?
There was a problem hiding this comment.
Ok but can we rename them?
There was a problem hiding this comment.
Sure, just not in this PR
| def test_dirichlet_infer_shape(M, size): | ||
| def test_dirichlet_infer_shape(make_args): | ||
| M_pt = iscalar("M") | ||
| test_values = {M_pt: 3} |
There was a problem hiding this comment.
This seems like overkill
| *rv_shape_val, rv_val = pytensor_fn( | ||
| *[ | ||
| i.tag.test_value | ||
| test_values[i] |
There was a problem hiding this comment.
nit: using i to represent a non-counter
There was a problem hiding this comment.
simplified a bunch of these tests in the last commit
ricardoV94
force-pushed
the
no_more_tag_test_value
branch
from
2d3f6f9 to
ab3439f
Compare
ricardoV94
force-pushed
the
no_more_tag_test_value
branch
from
ab3439f to
77ff77e
Compare
2 participants
Grunt work done by Claude
Closes #447
Closes #831
Get a clean cut with v3, instead of trying FutureWarning which was too onerous as seen in #831
As a reminder, the reason this is bad is that it permeates too many places. It's easy enough to setup a helper optimizer / graph walker that does this if someone needs it. I haven't personally ever used tag.test_values for debugging.
code diff: The hardest part to review on a skim is tests that insisted putting test values in the tag, to later extract and evaluate. I've been trying to remove these (numba backend used this non-stop in the past), but as you can see there were still a lot. The rest is just pure deletions