-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Unwrap features before passing them into a kernel #6807
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
Conversation
@pmeier can you provide time measurements on a datapipeline (e.g. image classification + AA + RandomErasing, #6681 (comment)) |
The diff that we are interested in: - 16747 0.206 0.000 0.255 0.000 /home/philip/git/pytorch/torchvision/torchvision/prototype/features/_feature.py:63(__torch_function__)
- 14487 0.022 0.000 0.022 0.000 {method 'as_subclass' of 'torch._C._TensorBase' objects}
+ 18896 0.032 0.000 0.032 0.000 {method 'as_subclass' of 'torch._C._TensorBase' objects} Meaning we are looking at a |
@pmeier by time measurements I meant torch benchmarks and not cprofile (which may be less reliable) |
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.
LGTM overall. Only one comment below. Also mypy seems to be failing due to an unused ignore.
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.
LGTM, thanks!
Summary: * unwrap features before calling the kernels * revert double unwrapping * cleanup * remove debug raise * more cleanup Reviewed By: YosuaMichael Differential Revision: D40588165 fbshipit-source-id: 3ce277bbe4f47124f572ca8cd185795b5917fe3e
Closes #6781 by going with the
self.as_subclass(torch.Tensor)
proposal. After this PR, there are no more__torch_function__
calls in the whole pipeline.In fact, it almost renders #6681 obsolete:
ndim
,device
, anddtype
are no longer accessed. Evenshape
is only accessed 4 times:vision/torchvision/prototype/transforms/functional/_meta.py
Lines 19 to 22 in 246de07
vision/torchvision/prototype/transforms/functional/_meta.py
Lines 40 to 41 in 246de07
vision/torchvision/prototype/transforms/functional/_meta.py
Lines 78 to 79 in 246de07
vision/torchvision/prototype/transforms/functional/_meta.py
Lines 91 to 92 in 246de07
Since they all only indirectly access
.shape
through properties we have anywayvision/torchvision/prototype/features/_image.py
Lines 105 to 111 in 246de07
vision/torchvision/prototype/features/_video.py
Lines 57 to 67 in 246de07
we could also opt to move the
with DisableTorchFunction()
context manager there and completely removevision/torchvision/prototype/features/_feature.py
Lines 132 to 152 in 246de07
cc @vfdev-5 @datumbox @bjuncek