PERF: Implement round on the block level #51498

lithomas1 · 2023-02-20T12:35:36Z

closes PERF: DataFrame.round() unnecessarily slow copared to np.round() #17254 (Replace xxxx with the GitHub issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

Benchmarks are showing same speedup as OP.

       before           after         ratio
     [c133327c]       [1a2c35f5]
     <main>           <faster-round>
-            115M              72M     0.63  frame_methods.Round.peakmem_round_transposed
-      1.24±0.2ms       60.9±0.9μs     0.05  frame_methods.Round.time_round
-        863±70ms       61.2±0.4μs     0.00  frame_methods.Round.time_round_transposed

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

pandas/core/frame.py

jbrockmendel · 2023-02-20T21:34:22Z

pandas/core/internals/blocks.py

+            Whether Copy on Write is enabled right now
+        """
+        if not self.is_numeric or self.is_bool:
+            return self.copy(deep=None if using_cow else True)


possibly raise?

I believe that DataFrame.round() never raises even when there are no numeric columns, but Series.round() does.

Do you want me to add a comment that this doesn't raise?

if this is just a perf PR than this should maintain behavior, but might merit an issue or TODO comment. looks like some of our EAs have round methods that are not documented as part of the interface.

…er-round

phofl · 2023-02-27T01:10:11Z

pandas/core/internals/blocks.py

+            Whether Copy on Write is enabled right now
+        """
+        if not self.is_numeric or self.is_bool:
+            return self.copy(deep=using_cow)


not using_cow I think?

Thanks for the catch.

phofl · 2023-02-27T21:46:36Z

pandas/core/internals/blocks.py

+        # error: Item "ExtensionArray" of "Union[ndarray[Any, Any], ExtensionArray]"
+        # has no attribute "round"
+        return new_block_2d(
+            self.values.round(decimals),  # type: ignore[union-attr]


I think you want to use self.make_block_same_class here? If class could change then self.make_block. Don't have to set placement there

This is dangerous, I don't think that round guarantees a copy, e.g.

arr = np.array([1, 2]) arr2 = arr.round(2) arr2[0] = 100

You can check this with

arr is arr2

(also please add a test for this case)

Edit: You can just pass the refs of self in this case like we do in other places

Thanks for the tip.
I found the source here https://github.com/numpy/numpy/blob/486878b37fc7439a3b2b87747f50db9b62fea8eb/numpy/core/src/multiarray/calculation.c#L625-L636
and it looks like this is only triggered for integer case with decimals >= 0.

BTW, it also looks like round is an operation that can be done inplace by using the out parameter in numpy.
Maybe I should open a PR for that as a follow-up?

Would have to check if it's actually faster, but if yes then should utilise it in CoW cases

Follow up sounds good to me

pandas/core/frame.py

phofl · 2023-02-28T23:00:45Z

pandas/core/internals/blocks.py

+        # returns the same array, so need to set refs
+        # https://github.com/numpy/numpy/blob/486878b37fc7439a3b2b87747f50db9b62fea8eb/numpy/core/src/multiarray/calculation.c#L625-L636
+        refs = None
+        if not self.is_extension and self.dtype.kind == "i" and decimals >= 0:


I'd prefer checking:

result = self.values.round(..) if self.values is result: refs = self.refs

This would not depend on an implementation detail of numpy

phofl · 2023-03-01T00:45:48Z

pandas/tests/copy_view/test_methods.py


    if using_copy_on_write:
        assert np.shares_memory(get_array(df2, "b"), get_array(df, "b"))
-        assert np.shares_memory(get_array(df2, "a"), get_array(df, "a"))
+        if decimals >= 0:
+            assert np.shares_memory(get_array(df2, "a"), get_array(df, "a"))


Can you do a else: assert not and add a todo that we can optimise with out?

phofl · 2023-03-01T00:45:59Z

pandas/tests/copy_view/test_methods.py

    if using_copy_on_write:
        assert not np.shares_memory(get_array(df2, "b"), get_array(df, "b"))
+        if decimals >= 0:


if think this if is not necessary? Both cases shouldn't share

phofl · 2023-03-01T00:46:19Z

pandas/core/internals/blocks.py

+                # https://github.com/numpy/numpy/blob/486878b37fc7439a3b2b87747f50db9b62fea8eb/numpy/core/src/multiarray/calculation.c#L625-L636
+                values = values.copy()
+        return self.make_block_same_class(
+            values,


This could fit into one line?

phofl

small comment, otherwise lgtm

lumberbot-app · 2023-03-01T22:54:22Z

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

git checkout 2.0.x
git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 f298507b153a166bdd2a919274e039c45e740c5d

You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #51498: PERF: Implement round on the block level'

Push to a named branch:

git push YOURFORK 2.0.x:auto-backport-of-pr-51498-on-2.0.x

Create a PR against branch 2.0.x, I would have named this PR:

"Backport PR #51498 on branch 2.0.x (PERF: Implement round on the block level)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

jbrockmendel · 2023-04-21T15:59:44Z

Belated: LGTM

PERF: Implement round on the block level

497c561

jbrockmendel reviewed Feb 20, 2023

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Feb 20, 2023

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

lithomas1 added 2 commits February 20, 2023 11:57

update

1a2c35f

update benchmarks

81d9ac6

jbrockmendel reviewed Feb 20, 2023

View reviewed changes

simonjayhawkins added the Performance Memory or execution speed performance label Feb 22, 2023

lithomas1 added 2 commits February 22, 2023 11:54

fix typing

1e6485c

Merge branch 'main' of https://github.com/pandas-dev/pandas into fast…

20e44e6

…er-round

lithomas1 marked this pull request as ready for review February 22, 2023 16:57

fix typing

f158438

lithomas1 requested a review from jbrockmendel February 23, 2023 21:21

phofl reviewed Feb 27, 2023

View reviewed changes

lithomas1 added 2 commits February 27, 2023 07:00

Update blocks.py

a472d79

Merge branch 'main' into faster-round

0997429

lithomas1 requested a review from phofl February 27, 2023 16:19

phofl reviewed Feb 27, 2023

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

lithomas1 added 2 commits February 28, 2023 17:46

address code review

f628b06

Merge branch 'main' into faster-round

300c19e

phofl reviewed Feb 28, 2023

View reviewed changes

address code review

d0fabca

phofl reviewed Mar 1, 2023

View reviewed changes

updates

15db500

phofl approved these changes Mar 1, 2023

View reviewed changes

lithomas1 added this to the 2.0 milestone Mar 1, 2023

lithomas1 merged commit f298507 into pandas-dev:main Mar 1, 2023

lithomas1 deleted the faster-round branch March 1, 2023 22:54

lumberbot-app bot added the Still Needs Manual Backport label Mar 1, 2023

lithomas1 added a commit to lithomas1/pandas that referenced this pull request Mar 1, 2023

Backport PR pandas-dev#51498: PERF: Implement round on the block level

76ae959

lithomas1 removed the Still Needs Manual Backport label Mar 1, 2023

phofl pushed a commit that referenced this pull request Mar 2, 2023

Backport PR #51498: PERF: Implement round on the block level (#51729)

7e61e3a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PERF: Implement round on the block level #51498

PERF: Implement round on the block level #51498

lithomas1 commented Feb 20, 2023 •

edited

Loading

jbrockmendel Feb 20, 2023

lithomas1 Feb 21, 2023

jbrockmendel Feb 21, 2023

phofl Feb 27, 2023

lithomas1 Feb 27, 2023

phofl Feb 27, 2023

phofl Feb 27, 2023 •

edited

Loading

lithomas1 Feb 28, 2023

phofl Feb 28, 2023

phofl Feb 28, 2023

phofl Feb 28, 2023

phofl Mar 1, 2023

phofl Mar 1, 2023

phofl Mar 1, 2023

phofl left a comment

lumberbot-app bot commented Mar 1, 2023

jbrockmendel commented Apr 21, 2023

PERF: Implement round on the block level #51498

PERF: Implement round on the block level #51498

Conversation

lithomas1 commented Feb 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phofl Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phofl left a comment

Choose a reason for hiding this comment

lumberbot-app bot commented Mar 1, 2023

jbrockmendel commented Apr 21, 2023

lithomas1 commented Feb 20, 2023 •

edited

Loading

phofl Feb 27, 2023 •

edited

Loading