Add PyTensor backend #362
Add PyTensor backend #362
Conversation
ricardoV94
force-pushed
the
pytensor_backend
branch
from
58831b5 to
95c9c3a
Compare
|
Ah you're running the CI on 3.8 which we don't support anymore :) |
ricardoV94
force-pushed
the
pytensor_backend
branch
from
95c9c3a to
fca5ff1
Compare
ricardoV94
force-pushed
the
pytensor_backend
branch
from
fca5ff1 to
65d8cad
Compare
|
Most imperative tests would work fine with PyTensor except for the use of |
ricardoV94
force-pushed
the
pytensor_backend
branch
from
1f5c250 to
1d0a538
Compare
| def to_numpy(self, x): | ||
| return x.eval() # Will only work if there are no symbolic inputs | ||
|
|
||
| def create_symbol(self, shape): |
There was a problem hiding this comment.
let's assume that shape is tuple/list. Does it break anything?
There was a problem hiding this comment.
There were tests where it was an integer. Needs to always be a sequence for us, that's why I added the check
| return self.pt.tensor(shape=shape) | ||
|
|
||
| def eval_symbol(self, symbol, input_dict): | ||
| # input_dict is actually a list of tuple? |
There was a problem hiding this comment.
it is dict: input symbol -> input value
There was a problem hiding this comment.
The tests always passed it as a list of tuples
There was a problem hiding this comment.
ah, correct. Some of backends' symbols were not hashable, need to rename this
| @@ -254,7 +255,7 @@ def test_functional_symbolic(): | |||
| ) | |||
| if predicted_out_data.shape != out_shape: | |||
| raise ValueError(f"Expected output shape {out_shape} but got {predicted_out_data.shape}") | |||
| assert np.testing.assert_array_almost_equal(predicted_out_data, expected_out_data, decimal=5) | |||
| np.testing.assert_array_almost_equal(predicted_out_data, expected_out_data, decimal=5) | |||
| frameworks: ['numpy pytorch tensorflow jax'] | ||
| frameworks: ['numpy pytorch tensorflow jax', 'pytensor'] | ||
| exclude: | ||
| - python-version: '3.8' |
There was a problem hiding this comment.
can you document which versions won't work and why?
There was a problem hiding this comment.
Yes, where would you like that, in the readme? A comment here? I always break yaml files whenever I try to put comments
There was a problem hiding this comment.
right in the file, there are some commens about testing of other frameworks above
need to think about this. Maybe we'll have to skip testing because of this, or use |
Not a blocker, I registered pytensor as symbolic so it's not picked by those tests. Just saying it would work fine because we can do graphs of constants. |
ricardoV94
force-pushed
the
pytensor_backend
branch
from
1d0a538 to
ea5d803
Compare
|
Tests are passing with the last pytensor release. Let me know if you want me to change anything else. Want me to rename the |
|
Looks good, I'll merge this.
NP, will rename in a separate PR. |
|
Any estimation for when a next release will be cut? |
|
I estimate this to happen a couple of hours ago :) Recommendation to add this to your CI so if you decide some changes around symbols (hashability/boolability/etc), you could confirm that einops integration is not broken. Tests are rather fast. |
|
Thanks 🙏 |
Hi!
I understand that the library is moving away from specific backends in favor of the array api standard, but this was so much easier for us to implement and test.
From a first glance the array api standard compatibility suite seems completely oblivious to lazy backends right now. Will double check and open an issue with them later if my suspicion is correct.
This also seems to test part of the test codebase that was untested since the drop of MxNet (?). Fixed two errors there.
Happy to remove PyTensor from the test suite if that's a drag / blocker. Not sure if we could test it from our CI as some of these test options are hardcoded in the library? Sounds like it should be easy to make them customizable though?
Currently all tests pass except the 3 tensor einsum multiplication. Issue opened in pymc-devs/pytensor#1184Should pass when it picks the new release