-
Notifications
You must be signed in to change notification settings - Fork 139
Implement pack/unpack helpers #1578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
36422cd
to
cf633e7
Compare
The pack -> type -> unpack -> replace pattern might be common enough to merit it's own helper. PyMC has tools for doing this, for example, in One other thing I forgot to mention is that this will all fail on inputs with shape 0, since that will ruin the |
da89b9d
to
9ead211
Compare
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (73.33%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1578 +/- ##
==========================================
- Coverage 81.63% 81.63% -0.01%
==========================================
Files 231 231
Lines 52993 53006 +13
Branches 9395 9397 +2
==========================================
+ Hits 43263 43272 +9
- Misses 7281 7283 +2
- Partials 2449 2451 +2
🚀 New features to boost your workflow:
|
2 and 3. I would really like to have these, it's what I needed for the batched_dot_to_core rewrites.This isn't a simple case of vectorize because the dims I want to pack are both on the left and right of other dims |
return join(axis, *bcast_tensor_inputs) | ||
|
||
|
||
def pack( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have a docstring (doctested) example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i will of course add docstrings. This was just to get a PR on the board and see your reaction to the 3 issues I raised. I didn't want to document everything before we decided on the final API
I am inclined to making this a core op and not just a helper. It obliviates most uses of reshape and it's much easier to reason about, not having to worry about pesky -1 or whether the reshape shape comes from the original input shapes or not. That would pretty much address #883 We could use OFG and/or specialize to reshape/split later. It need also not be done in this PR. It's an implementation detail as far as the user is concerned. |
9ead211
to
1649827
Compare
860a7ab
to
fcbd0af
Compare
Description
Adds pt.pack and pt.unpack helpers, roughly conforming to the
einops
functions of the same name.These helps are for situations where we have a ragged list of inputs that need to be raveled into a single flat list for some intermediate step. This occurs in places like optimization.
Example usage:
Unpack simply undoes the computation, although there's norewrite to ensure
pt.unpack(*pt.pack(*inputs))
is the identity function:The use-case I forsee is creating replacement for a function of the inputs we're packing, for example:
Note that the final compiled function depends only on
new_input
, only because the shapes of the 3 packed variables were statically known. This leads to my design choices section:pack
will eagerly return a list of integer shapes aspacked_shapes
if possible. If not possible, they will be symbolic shapes. This is maybe an anti-pattern -- we might prefer a rewrite to handle this later, but it seemed easy enough to do eagerly.pt.vectorize
.einops
API has arguments to support packing/unpacking on arbitrary subsets of dimensions. I didn't do this, because I couldn't think of a use-case that a user couldn't get himself usingDimShuffle
andvectorize
.Related Issue
Checklist
Type of change
📚 Documentation preview 📚: https://pytensor--1578.org.readthedocs.build/en/1578/