PERF: DataFrame.clip / Series.clip

[lukemanley]

• 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/v2.0.0.rst file if fixing a bug or adding a new feature.

Perf improvement in DataFrame.clip and Series.clip:

asv continuous -f 1.1 upstream/main perf-dataframe-clip -b frame_methods.Clip

       before           after         ratio
     [2070bb8d]       [39c44771]
     <main>           <perf-dataframe-clip>
-      14.1±0.4ms       11.2±0.4ms     0.79  frame_methods.Clip.time_clip('float64')
-         147±7ms         98.6±5ms     0.67  frame_methods.Clip.time_clip('Float64')
-       133±0.7ms         78.4±1ms     0.59  frame_methods.Clip.time_clip('float64[pyarrow]')

asv continuous -f 1.1 upstream/main perf-dataframe-clip -b series_methods.Clip

       before           after         ratio
     [2070bb8d]       [39c44771]
     <main>           <perf-dataframe-clip>
-        613±30μs         533±30μs     0.87  series_methods.Clip.time_clip(1000)
-        635±60μs          509±8μs     0.80  series_methods.Clip.time_clip(50)

[phofl]

The copy on write failures are legit. I guess you’ll have to do self.copy(deep=None) instead of self.copy()

[lukemanley]

The copy on write failures are legit. I guess you’ll have to do self.copy(deep=None) instead of self.copy()

I just tried that, still seems to fail. I made a slightly different change that seems to work, let me know if this looks ok.

[phofl]

This moves away from CoW syntax now:

Both objects should share memory, if clip is a no-op:

df = DataFrame({"a": [1.5, 2, 3]})
df_copy = df.copy()
arr_a = get_array(df, "a")
view = df[:]
df.clip(lower=1, inplace=True)
print(np.shares_memory(get_array(df, "a"), arr_a))

Also with inplace and without references, this can still modify the existing array inplace.

Your current pr avoids this (we don't have tests for this yet, because we did not get to it and we covered where extensively :))

Additionally, this introduces bugs when setting other dtypes:

df = DataFrame({"a": [1, 2, 3]})
df = df.clip(lower=1.5)

This is a no-op on this branch but works on main (should probably add a test)

In general, I'd prefer doing this on the block level. This keeps CoW handling in more or less on place.

[lukemanley]

Thanks for the explanation. I've updated the PR to use BlockManager.where and added a test for the int/float case you highlighted. I've updated the timings the OP.

[phofl]

Opened #51492 that covers possible cases. Would like to merge this first to ensure that we don't miss anything. Do you know why where is so much slower? Meaning the block method?

[phofl]

You could look into putmask for inplace ops, that avoids copying if no reference is found

[lukemanley]

You could look into putmask for inplace ops, that avoids copying if no reference is found

Added inplace via putmask.

Do you know why where is so much slower? Meaning the block method?

Yes, I think I'll open a separate PR for this. When we compute a boolean mask on an EA-backed frame, we get a "boolean" (EA) mask. This gets converted to an object dtype ndarray within BaseBlockManager.apply which is quite slow.

As an example:

import pandas as pd
import numpy as np

df = pd.DataFrame(np.random.randn(10**6, 2), dtype="Float64")

mask = df > 0

%timeit mask._values                                 # -> dtype object (SLOW...)
%timeit mask.to_numpy(dtype=bool, na_value=False)    # -> dtype bool

# 25.2 ms ± 408 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
# 539 µs ± 9.9 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

[phofl]

Merged the clip tests, could you rebase?

Ah that makes sense, got it.

[phofl]

small comments, otherwise lgtm

[lukemanley]

small comments, otherwise lgtm

@phofl - sorry, which small comments are you referring to?

[phofl]

Can we use > or >= in both cases? Also why don’t we need a mask in the inplace case?

[lukemanley]

The condition in putmask identifies the values to be updated so we look for values outside the threshold (e.g. ">")

The condition in where identifies the values to be left as is which are NA values (mask) and anything within the threshold (e.g. "<=")

So I think the difference is consistent with the meaning of those different methods. Using <= and >= in putmask would actually turn no-ops at the boundary into unnecessary ops I think.

[phofl]

Thx, makes sense. Did not consider that, could you add a really short comment to that effect?

[phofl]

Sorry looks like the did not get posted. Had some connection troubles today, reposted them

[phofl]

One last comment. Can you move to 2.1?

[phofl]

thx @lukemanley