-
Notifications
You must be signed in to change notification settings - Fork 651
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
PERF-#4705: Improve perf of arithmetic operations between Series
objects with shared .index
#4689
Conversation
Thanks for the PR @jbrockmendel ! Could you try running your performance measurements again with benchmark mode enabled? import modin.config as cfg
# Enable benchmark mode
cfg.BenchmarkMode.put(True) |
With BenchmarkMode enabled I get
|
@jbrockmendel could you try running the benchmarks for larger datasets and let us know how the performance looks there? Also, it looks like the rest of the CI runs aren't running because your commit message isn't formatted properly. You'll probably have to create a Github issue first and then link this PR to that. You can check this link out for more details: https://modin.readthedocs.io/en/stable/development/contributing.html |
Codecov Report
@@ Coverage Diff @@
## master #4689 +/- ##
==========================================
+ Coverage 85.25% 89.84% +4.59%
==========================================
Files 259 260 +1
Lines 19211 19494 +283
==========================================
+ Hits 16378 17515 +1137
+ Misses 2833 1979 -854
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
Updated OP with results on 10x larger DataFrame. |
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. @jbrockmendel could you please change the release notes?
Signed-off-by: Brock Mendel <jbrockmendel@gmail.com>
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.
Thanks @jbrockmendel !
Series
objects with shared .index
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.
@jbrockmendel, LGTM, thanks!
closes #4705