improve performance of {invert, solarize}_image_tensor #6819
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pmeier
commented
Oct 24, 2022
31 tasks
datumbox
approved these changes
Oct 24, 2022
There was a problem hiding this comment.
It's looking good. Perhaps address @vfdev-5 remark below, to ensure the previous and new behaviours are identical?
Worth noting that you also improve solarize which depends on invert. To get this speed gain, you would have to implement solarize again on v2, to call the new kernel. Do you plan to do this now or separately?
|
There is a related linter issue. |
pmeier
commented
Oct 24, 2022
| if image.dtype == torch.uint8: | ||
| return image.bitwise_not() | ||
| else: | ||
| return _FT._max_value(image.dtype) - image |
There was a problem hiding this comment.
I couldn't believe it, but this seems to be fastest way:
def scalar_sub(image):
return _max_value(image.dtype) - image
def new_full(image):
return image.new_full(image.shape, _max_value(image.dtype)).sub_(image)
def full_like(image):
return torch.full_like(image, _max_value(image.dtype)).sub_(image)[-------------------- invert float or signed integer tensors -------------------]
| scalar_sub | new_full | full_like
1 threads: ----------------------------------------------------------------------
(3, 256, 256) / int32 / cpu | 20 | 28 | 26
(3, 256, 256) / int32 / cuda | 5 | 8 | 7
(3, 256, 256) / int64 / cpu | 88 | 109 | 107
(3, 256, 256) / int64 / cuda | 13 | 22 | 22
(3, 256, 256) / float64 / cpu | 35 | 49 | 49
(3, 256, 256) / float64 / cuda | 12 | 21 | 21
(5, 3, 256, 256) / int32 / cpu | 76 | 112 | 110
(5, 3, 256, 256) / int32 / cuda | 34 | 67 | 67
(5, 3, 256, 256) / int64 / cpu | 439 | 510 | 524
(5, 3, 256, 256) / int64 / cuda | 68 | 133 | 133
(5, 3, 256, 256) / float64 / cpu | 140 | 215 | 220
(5, 3, 256, 256) / float64 / cuda | 68 | 133 | 133
Times are in microseconds (us).
Let's do it in a follow-up PR. Maybe there are more improvements possible. Will deal with it next. Edit: There is nothing to optimize in |
pmeier
changed the title
|
Re-run a reduced version of the benchmark with the recent changes: |
datumbox
reviewed
Oct 24, 2022
facebook-github-bot
pushed a commit
that referenced
this pull request
Oct 27, 2022
Summary: * improve performance of invert_image_tensor * cleanup * lint * more cleanup * use new invert in solarize Reviewed By: YosuaMichael Differential Revision: D40722898 fbshipit-source-id: b2157759d439a184be49d69b91e5ea82e2d96cc6
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Per title. The benchmark script from #6818 yields the following
uint8is much faster now, whilefloat32is on par with what we had before.cc @vfdev-5 @datumbox @bjuncek