Repository navigation
Conversation
7a38c78 to
bc4741e
Compare
bc4741e to
2a0760d
Compare
Assisted-by: AI
Assisted-by: AI
2a0760d to
9cc6089
Compare
|
Hi @yeetypete - Excited to see this, I hope we can unblock the issues you ran into. Trying to audit for re-exports is also on my radar, I had a few local commits to improve this but got sidetracked last week. It seems you're aware that the feature is not stable since you noted a regression in 1.3 for your uses. I actually do believe that the core type system is probably fairly stable now; at this point we have some numpy demos and pretty decent stubs for a good subset of jax + torch which gives me some confidence. But the stubs are quite unstable and incomplete as you noted, and I suspect there are bugs lurking in what we have as well. Thanks for all the PRs, at this point every time we experiment in typing a new codebase we run into gaps and I expect that will go on for a while. I'd love to hear how this is going for you / ideas about what we can do better. We've had good luck with a jax contributor so I know what we have can be useful for a library author, but you're in the position of being both a library author yourself and a user of torch. If you want to discuss anything specific that's in your way or could be better, a few options:
|
|
@stroxler Ah I see you've found the origin for some of the PRs, welcome! I'm experimenting with pyrefly type checking in this draft stack. Yeah we're aware that the tensor shape typing isn't quite stable yet but I've been keeping tabs on the progress. Now that tensor subclassing (we subclass torchvision's iiuc since this only impacts downstream consumers who are opting into pyrefly tensor shape checking when they consume vision3d as a dependency (and use pyrefly) the risk is low. The overall plan is:
In terms of tooling that would be nice to have:
Wdyt? You might see some stub PRs from @simonschlaepfer and @SBetschen as well who are working on vision3d with me :) Btw do the stubs get released at the same cadence as pyrefly or can we expect to get updates more quickly. We can also pin to github url commit sha in I'll join the discord and we can continue the discussion there :) |
|
The easy things to answer:
The harder question is whether you can apply stubs and not impact downstream users who aren't using shape types or Pyrefly. Right now the answer is probably no, because I think another type checker is going to detect the shape types and complain. That other type checker might wind up dropping the shape types and basically doing the right thing downstream, but I'm not confident this would work. It's a pretty brittle way to fall back. We've already had a question of how to annotate a library so that it works with other type checkers. At the moment, the only way to do it is to ship shape stubs that people can opt into (as we have with This is a significant design question, and I'm not sure we'll have a good solution right away. We can potentially have a discussion with interested folks on jax and numpy (@jorenham was asking similar questions about what it would take to put shape types directly into numpy without breaking non-Pyrefly users) about what seems best / most feasible. |
👍 we can definitely do this.
Ah ok thanks for letting me know. as long as the above works with the pyrefly LSP I think this would be ok as a workaround. Alternatively I guess we could do some build-time magic and autogenerate a stubs package which we release separately while stripping the pyrefly shape annotations when we build the wheel. But not sure if this is worth it or if anyone has tried something wild like this before 😂. We have no real intention of requiring the vision3d source itself to be parseable by other type checkers outside of pyrefly. In any case I think being able to annotate the shpes inline with existing type info rather than putting type annotations in a separate stub file is important to preserve. |
|
Stripping annotations might be an option if we can't figure out a cross-type-checker way of annotating libraries quickly enough (it's important but also pretty challenging to do this, so I don't know the timeline). It might not be that hard to do the actual stripping with something like LibCST. We could potentially build a little tool for it, in the pyrefly repo or elsewhere. The big downside would be the need to publish a stub package alongside the normal package. This is basically the mirror image of what we're doing with pyrefly-torch-stubs today, it could work but it's a little annoying. |
|
Version 1.4.0dev.1 of the stubs is up and has most of your fixes in, although I think not the last one we merged. It also probably fixes most of the re-export issues you ran into, although this stack already works around those problems. Any new issues on your side? I'm probably going to release 1.4.0dev.2 at some point this week. That should contain a rather large set of (mostly minor) torch bugfixes, plus I'm in the process of making int-arithmetic display improvements that should help a lot with convnets in particular |
|
I now have a proposal in facebook/pyrefly#5027 for a jaxtyping-esque syntax where Pyrefly types can be declared in what other type checkers see as I think this would give an option for using shape types without issues downstream, at the cost of heavier syntax. Whether this is better or worse than a stub-stripping tool I'm not sure, it depends how you feel about the syntax versus the complexity in shipping a separate stub package |
Unfortunately I haven't had the chance yet to give shape checking another spin with the merged fixes. This week I'll likely have some time.
Nice! I'll give this a try as well. It would be nice not to have to publish a separate package with the stubs. Does the implementation mean we need to depend on |
You'd still need FWIW I haven't actually made a production-worthy version and merged this feature yet, so you could test it but you'd have to use a pyrefly + shape_extensions built off of that draft PR. That said, the feedback so far has been positive so I do think I will be moving forward with this in the next week or two. |
No worries, we'll give a version built from your branch a try. @SBetschen will take over some testing here 🚀 |
Enable Pyrefly's tensor shape checking through the
pyrefly-torch-stubsdev dependency. Additionally:torchvision.tv_tensors.TVTensorandImageunderstubs/, and declare the rank ofPointCloud3D,BoundingBoxes3D, and the camera types through aTYPE_CHECKINGbase alias.torch.testing,torch.linalg, ...) since the partial stub package no longer re-exports them.torch.rand(()),new_empty(n, m),torch.nonzero(..., as_tuple=True),as_subclass.CONTRIBUTING.md.Assisted-by: AI