PERF: Avoid np.divmod in maybe_sequence_to_range #57812

mroeschke · 2024-03-11T18:34:44Z

Made a is_range method (like is_range_indexer) that avoids a np.divmod operation

In [1]: from pandas import *; import numpy as np
   ...: np.random.seed(123)
   ...: size = 1_000_000
   ...: ngroups = 1000
   ...: data = Series(np.random.randint(0, ngroups, size=size))
+ /opt/miniconda3/envs/pandas-dev/bin/ninja
[1/1] Generating write_version_file with a custom command

In [2]: %timeit data.groupby(data).groups
14 ms ± 552 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)  # PR

In [3]: %timeit data.groupby(data).groups
17.8 ms ± 84.8 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)  # main

jbrockmendel · 2024-03-11T23:08:16Z

pandas/_libs/lib.pyx

@@ -678,6 +678,26 @@ def is_range_indexer(ndarray[int6432_t, ndim=1] left, Py_ssize_t n) -> bool:
    return True


+@cython.wraparound(False)
+@cython.boundscheck(False)


Don’t we only need int64?

Sometimes we'll be calling self._shallow_copy(Index[int]._values) so I suppose the int type could be non-int64

Maybe should just use np.intp ? That matches what range would use internally

Looks like np.intp didn't work for the 32 bit and Windows build 0b484bd

Are you seeing build failures just doing what @jbrockmendel suggests with int64 then? The error message ValueError: Buffer dtype mismatch, expected 'intp_t' but got 'long long' on the 32 bit build would seemingly indicate the 32 bit part of the fused type is unused, since long long is by definition at least 64 bits

https://github.com/pandas-dev/pandas/actions/runs/8268872377/job/22622796389#step:4:43677

With just int64_t, 32 bit and Windows builds are failing with ValueError: Buffer dtype mismatch, expected 'int64_t' but got 'long' https://github.com/pandas-dev/pandas/actions/runs/8284194543/job/22669177062?pr=57812

pandas/_libs/lib.pyx

WillAyd · 2024-03-13T02:43:14Z

pandas/_libs/lib.pyx

@@ -678,6 +678,26 @@ def is_range_indexer(ndarray[int6432_t, ndim=1] left, Py_ssize_t n) -> bool:
    return True


+@cython.wraparound(False)
+@cython.boundscheck(False)


Maybe should just use np.intp ? That matches what range would use internally

jbrockmendel · 2024-03-13T03:10:51Z

pandas/_libs/lib.pyx

@@ -678,6 +678,26 @@ def is_range_indexer(ndarray[int6432_t, ndim=1] left, Py_ssize_t n) -> bool:
    return True


+@cython.wraparound(False)
+@cython.boundscheck(False)
+def is_range(ndarray[int6432_t, ndim=1] sequence, int64_t diff) -> bool:


can this have a more verbose-but-descriptive name? from this name alone id expect this to be a somehow-optimized isinstance(sequence, range)

Sure thing. Renamed to is_sequence_range

…r/step

This reverts commit b8ea98c.

…r/step

mroeschke · 2024-03-20T23:55:57Z

Any other feedback here @jbrockmendel @WillAyd?

WillAyd

lgtm

jbrockmendel · 2024-03-21T15:41:19Z

pandas/_libs/lib.pyx

+
+    for i in range(n):
+
+        if sequence[i] != sequence[0] + i * step:


could get slight improvement by accessing sequence[0] just once outside the loop?

Good idea. I'll implement this

…r/step

* PERF: Avoid np.divmod in RangeIndex._shallow_copy * Make is_range * pyi error * Use step * Switch back to int6432 * try int64_t * Revert "try int64_t" This reverts commit b8ea98c. * Adjust maybe_sequence_to_range * Access first element once

PERF: Avoid np.divmod in RangeIndex._shallow_copy

9b18b6b

mroeschke added Performance Memory or execution speed performance Index Related to the Index class or subclasses labels Mar 11, 2024

mroeschke requested a review from WillAyd as a code owner March 11, 2024 18:34

mroeschke added 2 commits March 11, 2024 12:48

Make is_range

9bcc4ea

pyi error

c9339d5

mroeschke added this to the 3.0 milestone Mar 11, 2024

jbrockmendel reviewed Mar 11, 2024

View reviewed changes

WillAyd reviewed Mar 13, 2024

View reviewed changes

jbrockmendel reviewed Mar 13, 2024

View reviewed changes

mroeschke added 8 commits March 13, 2024 10:15

Merge remote-tracking branch 'upstream/main' into ref/is_range_indexe…

1e98a41

…r/step

Use step

0b484bd

Merge remote-tracking branch 'upstream/main' into ref/is_range_indexe…

981e3c9

…r/step

Switch back to int6432

997416f

Merge remote-tracking branch 'upstream/main' into ref/is_range_indexe…

d3ae4e9

…r/step

try int64_t

b8ea98c

Revert "try int64_t"

21f13d9

This reverts commit b8ea98c.

Merge remote-tracking branch 'upstream/main' into ref/is_range_indexe…

2c77071

…r/step

mroeschke mentioned this pull request Mar 15, 2024

PERF: Remove slower short circut checks in RangeIndex__getitem__ #57856

Merged

1 task

mroeschke added 3 commits March 19, 2024 10:03

Merge remote-tracking branch 'upstream/main' into ref/is_range_indexe…

3daca65

…r/step

Merge remote-tracking branch 'upstream/main' into ref/is_range_indexe…

339a914

…r/step

Adjust maybe_sequence_to_range

503d96f

mroeschke changed the title ~~PERF: Avoid np.divmod in RangeIndex._shallow_copy~~ PERF: Avoid np.divmod in maybe_sequence_to_range Mar 20, 2024

WillAyd approved these changes Mar 21, 2024

View reviewed changes

jbrockmendel reviewed Mar 21, 2024

View reviewed changes

mroeschke added 2 commits March 21, 2024 09:07

Merge remote-tracking branch 'upstream/main' into ref/is_range_indexe…

58a6f27

…r/step

Access first element once

5f31dd8

mroeschke merged commit bfaf917 into pandas-dev:main Mar 21, 2024
46 checks passed

mroeschke deleted the ref/is_range_indexer/step branch March 21, 2024 17:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PERF: Avoid np.divmod in maybe_sequence_to_range #57812

PERF: Avoid np.divmod in maybe_sequence_to_range #57812

mroeschke commented Mar 11, 2024 •

edited

Loading

jbrockmendel Mar 11, 2024

mroeschke Mar 11, 2024

WillAyd Mar 13, 2024

mroeschke Mar 13, 2024

WillAyd Mar 14, 2024

mroeschke Mar 14, 2024 •

edited

Loading

WillAyd Mar 13, 2024

jbrockmendel Mar 13, 2024

mroeschke Mar 13, 2024

mroeschke commented Mar 20, 2024

WillAyd left a comment

jbrockmendel Mar 21, 2024

mroeschke Mar 21, 2024

PERF: Avoid np.divmod in maybe_sequence_to_range #57812

PERF: Avoid np.divmod in maybe_sequence_to_range #57812

Conversation

mroeschke commented Mar 11, 2024 • 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

mroeschke Mar 14, 2024 • 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

mroeschke commented Mar 20, 2024

WillAyd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mroeschke commented Mar 11, 2024 •

edited

Loading

mroeschke Mar 14, 2024 •

edited

Loading