Speed up torch.unique_consecutive() #64835
Closed
Conversation
This file contains hidden or 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
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 89e6648 (more details on the Dr. CI page):
🕵️ 3 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
Codecov Report
@@ Coverage Diff @@
## master #64835 +/- ##
==========================================
+ Coverage 66.65% 66.68% +0.03%
==========================================
Files 714 714
Lines 92546 92598 +52
==========================================
+ Hits 61685 61752 +67
+ Misses 30861 30846 -15 |
ngimel
reviewed
Sep 11, 2021
ngimel
added
the
triaged
oToToT
commented
Sep 12, 2021
aten/src/ATen/native/Unique.cpp
Outdated
| std::tuple<Tensor, Tensor, Tensor> | ||
| unique_consecutive_cpu(const Tensor& self, const bool return_inverse, const bool return_counts, c10::optional<int64_t> dim) { | ||
| if (!dim.has_value()) { | ||
| if (!dim.has_value() || (dim.value() == 0 && self.sizes().size() == 1)) { |
There was a problem hiding this comment.
Without this check, the result would be
torch.unique_consecutive(dim=0) time: 16.88133478164673
torch.unique_consecutive() time: 0.024564027786254883
torch.unique_consecutive(dim=0) time: 28.210450410842896
torch.unique_consecutive(dim=1) time: 0.30884695053100586
As you could see, this check slightly slows down torch.unique_consecutive(dim=1), but it could also speed up torch.unique_consecutive(dim=0) very much.
Hence, I still leave it here.
There was a problem hiding this comment.
How can this check affect dim=1 case? With or without this check dim=1 case should go to unique_dim_consecutive_cpu in line 287. 5x slowdown (from 0.3 to 1.6) seems significant, why is your algo slower than the previous one for dim=1?
There was a problem hiding this comment.
Yes, I believe it should always go to unique_dim_consecutive_cpu in this case.
Therefore, I am also curious about this situation. I didn't expect a huge differences at first.
Maybe it is some CPU branching issue?
If I tried loop the last case 5 times, the result become
torch.unique_consecutive(dim=0) time: 0.06868529319763184
torch.unique_consecutive() time: 0.06931805610656738
torch.unique_consecutive(dim=0) time: 26.346323251724243
torch.unique_consecutive(dim=1) time: 1.2163631916046143
torch.unique_consecutive(dim=1) time: 0.2443101406097412
torch.unique_consecutive(dim=1) time: 0.24067211151123047
torch.unique_consecutive(dim=1) time: 0.25301051139831543
torch.unique_consecutive(dim=1) time: 0.24738121032714844
I will try torch.utils.benchmark or something else to get a more thorough analysis if possible.
There was a problem hiding this comment.
Ah ok, it seems like a warm-up issue then, but please post torch.utils.benchmark timings here, so that we have a record.
There was a problem hiding this comment.
Summary
For 1d case, >100x more faster on dim=0 case with (dim.value() == 0 && self.sizes().size() == 1).
For 2d case, there is no significant impact with or without (dim.value() == 0 && self.sizes().size() == 1).
In general, the new implementation (89e6648) is 3x more faster than the original implementation (be09195).
Result
The script I test test_1d.py, test_2d.py.
ngimel
approved these changes
Sep 13, 2021
|
@ngimel has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
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.
Fixes #62690
Summary
Like the way
unique_consecutive_cpu_templateimplemented, this PR reimplements_unique_dim_cpu_implto get better performance.Also, because the overhead of
unique_dim_consecutive_cpuis quite large, directly callunique_consecutive_cpu_templatewhen we know the given input is a 1d-array.Benchmark
Script
Before
After
System Information