-
-
Notifications
You must be signed in to change notification settings - Fork 403
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
Optimize downsample1d when data is shared between layers #6075
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6075 +/- ##
==========================================
- Coverage 88.65% 88.54% -0.12%
==========================================
Files 314 314
Lines 65942 65972 +30
==========================================
- Hits 58462 58414 -48
- Misses 7480 7558 +78
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@philippjfr, this should be ready for review; I can't request you review it as you are the original author of the issue. |
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.
Can't review it, but your changes and the tests look good!
def test_downsample1d_shared_data(): | ||
runs = [0] | ||
|
||
class mocksample(downsample1d): |
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.
Probably would have used unittest.mock.patch
on the Interface but this is fine.
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Another optimization for
downsample1d
when multiple layers share the same data. Since the majority of time is spent slicing the data we can apply an optimization when all elements in an (Nd)Overlay that is being downsampled share the same underlying data and are indexed along the same key dimension(s).In this case we simply get the first element slice it with the current x_range and then pass it down to each separate
self._process
call.Here's the test case I was using:
Results before:
Results after:
Addresses part of #6058
Ping @droumis and @hoxbro