-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Expose StreamBase and EventBase #126257
Expose StreamBase and EventBase #126257
Conversation
5748b30
to
b0749d3
Compare
I did some check in fbcode and the blast radius does not seem too big |
If we are careful not to break the existing extensions relying on the old class names, it should work in general. On the other hand, such reliance indicates that they should indeed be exposed as public classes. |
I don't think you need this. The shared class here are |
So you also think they should be moved to torch.types? If both of you think it is a better way to organize code, I will do it. |
I'm afraid I don't know what |
@cyyever is talking about the module, torch/types.py |
Ho I missed that one, thanks! My expectation is that you can use torch.Stream and torch.Event as type hints. Since they are already available in our public API, we don't need any special entry for them in torch.types. |
@albanD Their class names contains an underscore prefix before, which causing pylint and mypy complaining because that is the convention of inner classes. The main fix here is just to delete the underscore prefix. |
I am not sure what you mean? |
To be clear, these two APIs were added recently as the way to support device-generic stream/event. You can check the mro for any stream/event-like object we have and they should all properly contain torch.Stream / torch.Event. |
So that we can use them in type hinting to avoid device-specific types like CUDAStream and XPUStream.
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang