Skip to content

[PERF] Get rid of MultiIndex conversion in IntervalIndex.is_monotonic methods #25820

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Apr 21, 2019
Merged

[PERF] Get rid of MultiIndex conversion in IntervalIndex.is_monotonic methods #25820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7cb99e1
Get rid of _multiindex
makbigc Mar 19, 2019
0c54e55
Add benchmark test
makbigc Mar 19, 2019
e9f2601
Decrease test_monotonic_inc by one order and change is_monotonic
makbigc Mar 21, 2019
8c5c580
Remove the exception for the mixed type
makbigc Mar 24, 2019
543b0d3
Add a whatsnew note
makbigc Apr 6, 2019
0bba602
Use algos.is_monotonic instead of Index instant
makbigc Apr 6, 2019
ceb91f3
Add one more bench
makbigc Apr 6, 2019
2e6fa28
Move is_monotonic_inc to IntervalTree
makbigc Apr 14, 2019
34ad0f2
Run _engine method in setup
makbigc Apr 14, 2019
57f8404
Remove trailing space
makbigc Apr 15, 2019
480b353
Merge master
makbigc Apr 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion asv_bench/benchmarks/index_object.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import numpy as np
import pandas.util.testing as tm
from pandas import (Series, date_range, DatetimeIndex, Index, RangeIndex,
Float64Index)
Float64Index, IntervalIndex)


class SetOperations:
Expand Down Expand Up @@ -181,4 +181,18 @@ def time_get_loc(self):
self.ind.get_loc(0)


class IntervalIndexMethod(object):
# GH 24813
params = [10**3, 10**5]

def setup(self, N):
left = np.append(np.arange(N), np.array(0))
right = np.append(np.arange(1, N + 1), np.array(1))
self.intv = IntervalIndex.from_arrays(left, right)
self.intv._engine

def time_monotonic_inc(self, N):
self.intv.is_monotonic_increasing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just note that this measures access speed to the cached value, not the time cost of the underlying operation. To time the underlying method, you need to do pd.IntervalIndex.is_monotonic_increasing.func(self.intv), but that hits a cached value at self.intv._multiindex.is_monotonic_increasing in master, so will still seem to be in the nanosecond range, but really won't be because it's just the caching doing tricks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. The performance is even better. The _engine is set in prior in asv setup method.

All benchmarks:

       before           after         ratio
     [a1fee919]       [6fb6d2cb]
     <master>         <GH24813-inc-dec>
          229±1ns          209±1ns     0.91  index_object.IntervalIndexMethod.time_monotonic_inc(1000)
-        450±10ns          260±2ns     0.58  index_object.IntervalIndexMethod.time_monotonic_inc(100000)



from .pandas_vb_common import setup # noqa: F401
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.25.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ Performance Improvements
- Improved performance of :meth:`pandas.core.groupby.GroupBy.quantile` (:issue:`20405`)
- Improved performance of :meth:`read_csv` by faster tokenizing and faster parsing of small float numbers (:issue:`25784`)
- Improved performance of :meth:`read_csv` by faster parsing of N/A and boolean values (:issue:`25804`)
- Imporved performance of :meth:`IntervalIndex.is_monotonic`, :meth:`IntervalIndex.is_monotonic_increasing` and :meth:`IntervalIndex.is_monotonic_decreasing` by removing conversion to :class:`MultiIndex` (:issue:`24813`)
- Improved performance of :meth:DataFrame.`to_csv` when write datetime dtype data (:issue:`25708`)
- Improved performance of :meth:`read_csv` by much faster parsing of MM/YYYY and DD/MM/YYYY datetime formats (:issue:`25922`)

Expand Down
13 changes: 13 additions & 0 deletions pandas/_libs/intervaltree.pxi.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Template for intervaltree
WARNING: DO NOT edit .pxi FILE directly, .pxi is generated from .pxi.in
"""

from pandas._libs.algos import is_monotonic

ctypedef fused scalar_t:
float64_t
float32_t
Expand Down Expand Up @@ -101,6 +103,17 @@ cdef class IntervalTree(IntervalMixin):

return self._is_overlapping

@property
def is_monotonic_increasing(self):
"""
Return True if the IntervalTree is monotonic increasing (only equal or
increasing values), else False
"""
values = [self.right, self.left]

sort_order = np.lexsort(values)
return is_monotonic(sort_order, False)[0]

def get_loc(self, scalar_t key):
"""Return all positions corresponding to intervals that overlap with
the given scalar key
Expand Down
6 changes: 3 additions & 3 deletions pandas/core/indexes/interval.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,23 +438,23 @@ def is_monotonic(self):
Return True if the IntervalIndex is monotonic increasing (only equal or
increasing values), else False
"""
return self._multiindex.is_monotonic
return self.is_monotonic_increasing

@cache_readonly
def is_monotonic_increasing(self):
"""
Return True if the IntervalIndex is monotonic increasing (only equal or
increasing values), else False
"""
return self._multiindex.is_monotonic_increasing
return self._engine.is_monotonic_increasing

@cache_readonly
def is_monotonic_decreasing(self):
"""
Return True if the IntervalIndex is monotonic decreasing (only equal or
decreasing values), else False
"""
return self._multiindex.is_monotonic_decreasing
return self[::-1].is_monotonic_increasing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer checking monotonic_decreasing directly, so:

values = [self.right, self.left]
sort_order = np.lexsort(values)
return is_monotonic(sort_order, False)[1]

(But adding the properties to IntervalTree is even better).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried in this way. But is_monotonic(sort_order, False)[1] returns False for stationary IntervalIndex, e.g. IntervalIndex([(0, 1], (0, 1]], closed='right', dtype='interval[int64]'). By definition, is_monotonic_decreasing should be True for equal values.

is_monotonic(sort_order, False)[0] returns True for the stationary IntervalIndex.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, as lexsort does treats equal values as increasing their index location, so is_monotonic(...)[1] will fail.

It would still deliver efficiency gains from calculating is_monotonic_increasing and unique simultaneous using is_monotonic in IntervalTree as described above. Would you be up for adding this? Else this PR is good IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When implementing is_unique in this way, I got a similar problem. That is, is_monotonic(sort_order, False)[2] returns False for an IntervalIndex with consecutively identity element, e.g. IntervalIndex([(0, 1], (0, 1], (2, 3]]) .

IntervalIndex.is_unique is being handled in #25159


@cache_readonly
def is_unique(self):
Expand Down