PERF: Implement get_freq_code in cython frequencies #17422

jbrockmendel · 2017-09-02T17:50:06Z

There's a whole bunch of tseries.frequencies and tseries.offsets that should be moved into cython to improve Period (and PeriodIndex, and to a lesser extend Timestamp) performance. This PR just starts with the function get_freq_code`.

To keep the PR small, there are a bunch of things it doesnt do that can further improve things in follow-ups:

Nothing is cdef'd or declared in a pxd file
_libs.period still gets get_freq_code from tseries.frequencies instead of directly from tslibs.frequencies
A handful of now-duplicate constants in tseries.frequencies are not removed.

In [6]: per = pd.Period.now('min')
In [7]: %timeit per.month

Before:

The slowest run took 15.03 times longer than the fastest. This could mean that an intermediate result is being cached.
1000000 loops, best of 3: 1.4 µs per loop

After:

The slowest run took 25.64 times longer than the fastest. This could mean that an intermediate result is being cached.
1000000 loops, best of 3: 744 ns per loop

closes #xxxx
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

codecov · 2017-09-02T23:10:55Z

Codecov Report

Merging #17422 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #17422      +/-   ##
==========================================
- Coverage   91.15%   91.13%   -0.03%     
==========================================
  Files         163      163              
  Lines       49581    49552      -29     
==========================================
- Hits        45198    45161      -37     
- Misses       4383     4391       +8

Flag	Coverage Δ
#multiple	`88.92% <100%> (-0.01%)`	⬇️
#single	`40.24% <100%> (-0.08%)`	⬇️

Impacted Files	Coverage Δ
pandas/tseries/frequencies.py	`96.31% <100%> (-0.2%)`	⬇️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.72% <0%> (-0.1%)`	⬇️
pandas/core/indexes/datetimes.py	`95.43% <0%> (+0.09%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1981b67...f48c5c3. Read the comment docs.

codecov · 2017-09-02T23:11:02Z

Codecov Report

Merging #17422 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #17422      +/-   ##
==========================================
- Coverage   91.15%   91.14%   -0.02%     
==========================================
  Files         163      163              
  Lines       49581    49561      -20     
==========================================
- Hits        45198    45171      -27     
- Misses       4383     4390       +7

Flag	Coverage Δ
#multiple	`88.92% <100%> (-0.01%)`	⬇️
#single	`40.23% <100%> (-0.09%)`	⬇️

Impacted Files	Coverage Δ
pandas/tseries/frequencies.py	`96.31% <100%> (-0.2%)`	⬇️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/io/parquet.py	`65.38% <0%> (-0.37%)`	⬇️
pandas/core/frame.py	`97.72% <0%> (-0.1%)`	⬇️
pandas/core/generic.py	`91.98% <0%> (-0.01%)`	⬇️
pandas/core/indexes/base.py	`96.29% <0%> (ø)`	⬆️
pandas/core/indexes/multi.py	`96.9% <0%> (ø)`	⬆️
pandas/io/parsers.py	`95.46% <0%> (ø)`	⬆️
pandas/plotting/_core.py	`82.69% <0%> (+0.01%)`	⬆️
pandas/core/indexes/datetimes.py	`95.52% <0%> (+0.19%)`	⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1981b67...77e2e75. Read the comment docs.

jreback

if u are claiming. perf boost then pls add asv

jreback · 2017-09-02T23:30:15Z

pandas/_libs/tslibs/frequencies.pyx

+from util cimport is_integer_object
+
+
+@cython.ccall


what does this decorator do?

@cython.ccall def foo(x):

is equivalent to cpdef foo(x):

then use cpdef foo(x); don't try to change the existing style in a single PR.

jbrockmendel · 2017-09-03T02:02:08Z

asv is throwing build-time errors. I've filed an issue there. For now the timeit results will have to do.

jreback

i still want actual asv code

jbrockmendel · 2017-09-03T05:59:50Z

@jorisvandenbossche Any chance you want to play "good cop" here? I could use some help with asv, see error discussion here.

jreback · 2017-09-03T13:38:16Z

@jbrockmendel you need to add asv benchmarks to he code you are pushing

you are touching highly perf sensistive code and it is very easy to break things
think of these like tests

jbrockmendel · 2017-09-03T18:32:37Z

Just pushed a commit that adds asv checks for Period property lookups. I can't get asv to run it, would appreciate it if someone else would give it a shot.

jreback · 2017-09-05T10:31:43Z

pandas/_libs/tslibs/frequencies.pyx

+from util cimport is_integer_object
+
+
+@cython.ccall


then use cpdef foo(x); don't try to change the existing style in a single PR.

jbrockmendel · 2017-09-05T14:18:47Z

then use cpdef foo(x); don't try to change the existing style in a single PR.

Note that this code was moved from non-cython, so is not changing existing style. The idea was that where possible, retaining pure-python compatibility would be nice. But not a big deal, so will change.

jbrockmendel · 2017-09-06T15:37:24Z

Looks like Travis error is an unrelated HTTP timeout.

jreback · 2017-09-07T01:11:34Z

pandas/_libs/tslibs/frequencies.pyx

+
+import numpy as np
+cimport numpy as np
+np.import_array()  # Without this, `is_integer_object` causes segfaults


remove the comment, this is standard to do.

jreback · 2017-09-07T01:12:50Z

ok this looks good. just remove that comment and push again to get everything passing. ping on green.

jbrockmendel · 2017-09-07T05:09:47Z

ping.

jreback · 2017-09-07T11:32:37Z

https://travis-ci.org/pandas-dev/pandas/jobs/272723119 is failing. FYI just because they are in the allowed failing section doesn't mean they actually should fail. this is the build test, which installs like a user, w/o a development build.

jreback · 2017-09-07T11:33:42Z

setup.py

@@ -492,6 +493,8 @@ def pxd(name):
                     'sources': ['pandas/_libs/src/datetime/np_datetime.c',
                                 'pandas/_libs/src/datetime/np_datetime_strings.c',
                                 'pandas/_libs/src/period_helper.c']},
+    '_libs.tslibs.frequencies': {'pyxfile': '_libs/tslibs/frequencies',


this also needs to be listed in the setup function, IOW all sub-dirs have to be listed, put it after pandas._libs

jreback

see comments

jbrockmendel · 2017-09-07T17:59:35Z

Looks like the allowed-fail mentioned earlier is now OK, but the 3.5 OSX is now failing, says it just stalled and received no output for 10 minutes. The only change since last night's all-green is the entry in the setup.py packages.

Implement .pxd file for tslibs.frequencies cimport get_freq_code in period.pyx Replace isinstance checks in period.pyx with C-equivalents form util Remove unused imports from period.pyx Remove code in tseries.frequencies that can now be imported from tslibs.frequencies

jbrockmendel added 2 commits September 2, 2017 10:31

Implement get_freq_code cython frequencies

63b0a2b

flake8 fixup

f48c5c3

jreback requested changes Sep 2, 2017

View reviewed changes

asv benchmark for Period property lookups

83d6781

jreback requested changes Sep 3, 2017

View reviewed changes

gfyoung added Clean Internals Related to non-user accessible pandas implementation labels Sep 3, 2017

sinhrks added the Frequency DateOffsets label Sep 4, 2017

jreback requested changes Sep 5, 2017

View reviewed changes

Per reviewer request, change @cython.ccall-->cpdef

3ab53ff

jreback reviewed Sep 7, 2017

View reviewed changes

jreback added this to the 0.21.0 milestone Sep 7, 2017

jreback approved these changes Sep 7, 2017

View reviewed changes

jreback changed the title ~~Implement get_freq_code in cython frequencies~~ PERF: Implement get_freq_code in cython frequencies Sep 7, 2017

jreback added the Performance Memory or execution speed performance label Sep 7, 2017

jreback mentioned this pull request Sep 7, 2017

Remove unnecessary iNaT checks from _Period properties #17421

Merged

4 tasks

Remove comment per reviewer request

150db3c

jreback reviewed Sep 7, 2017

View reviewed changes

jreback requested changes Sep 7, 2017

View reviewed changes

jreback mentioned this pull request Sep 7, 2017

Separate out strptime.pyx from tslib #17342

Merged

4 tasks

Add tslibs to setup.py packages

77e2e75

jreback approved these changes Sep 8, 2017

View reviewed changes

jreback merged commit 7e4e8ac into pandas-dev:master Sep 8, 2017

jreback pushed a commit that referenced this pull request Sep 8, 2017

Follow up to #17422 (#17472)

d6df8ea

jbrockmendel added a commit to jbrockmendel/pandas that referenced this pull request Sep 10, 2017

PERF: Implement get_freq_code in cython frequencies (pandas-dev#17422)

72da858

jbrockmendel deleted the cy_freqs branch October 30, 2017 16:25

alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017

PERF: Implement get_freq_code in cython frequencies (pandas-dev#17422)

239acb2

alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017

Follow up to pandas-dev#17422 (pandas-dev#17472)

2872e03

No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017

PERF: Implement get_freq_code in cython frequencies (pandas-dev#17422)

075b868

No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017

Follow up to pandas-dev#17422 (pandas-dev#17472)

5dac3d5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PERF: Implement get_freq_code in cython frequencies #17422

PERF: Implement get_freq_code in cython frequencies #17422

jbrockmendel commented Sep 2, 2017

codecov bot commented Sep 2, 2017

codecov bot commented Sep 2, 2017 •

edited

Loading

jreback left a comment

jreback Sep 2, 2017

jbrockmendel Sep 3, 2017

jreback Sep 5, 2017

jbrockmendel commented Sep 3, 2017

jreback left a comment

jbrockmendel commented Sep 3, 2017

jreback commented Sep 3, 2017

jbrockmendel commented Sep 3, 2017

jreback Sep 5, 2017

jbrockmendel commented Sep 5, 2017

jbrockmendel commented Sep 6, 2017

jreback Sep 7, 2017

jreback commented Sep 7, 2017

jbrockmendel commented Sep 7, 2017

jreback commented Sep 7, 2017

jreback Sep 7, 2017

jreback left a comment

jbrockmendel commented Sep 7, 2017

PERF: Implement get_freq_code in cython frequencies #17422

PERF: Implement get_freq_code in cython frequencies #17422

Conversation

jbrockmendel commented Sep 2, 2017

codecov bot commented Sep 2, 2017

Codecov Report

codecov bot commented Sep 2, 2017 • edited Loading

Codecov Report

jreback left a comment

Choose a reason for hiding this comment

jreback Sep 2, 2017

Choose a reason for hiding this comment

jbrockmendel Sep 3, 2017

Choose a reason for hiding this comment

jreback Sep 5, 2017

Choose a reason for hiding this comment

jbrockmendel commented Sep 3, 2017

jreback left a comment

Choose a reason for hiding this comment

jbrockmendel commented Sep 3, 2017

jreback commented Sep 3, 2017

jbrockmendel commented Sep 3, 2017

jreback Sep 5, 2017

Choose a reason for hiding this comment

jbrockmendel commented Sep 5, 2017

jbrockmendel commented Sep 6, 2017

jreback Sep 7, 2017

Choose a reason for hiding this comment

jreback commented Sep 7, 2017

jbrockmendel commented Sep 7, 2017

jreback commented Sep 7, 2017

jreback Sep 7, 2017

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

jbrockmendel commented Sep 7, 2017

codecov bot commented Sep 2, 2017 •

edited

Loading