Skip to content
This repository was archived by the owner on Feb 2, 2024. It is now read-only.

Optimize series.rolling.sum() - #608

Merged
AlexanderKalistratov merged 17 commits into
IntelPython:masterfrom
densmirn:feature/series_rolling_sum_opt
Feb 20, 2020
Merged

Optimize series.rolling.sum()#608
AlexanderKalistratov merged 17 commits into
IntelPython:masterfrom
densmirn:feature/series_rolling_sum_opt

Conversation

@densmirn

Copy link
Copy Markdown
Contributor

Previous implementation results:

namenthreadstypesizemedian
Series.rolling.sum4Python100000000.616
Series.rolling.sum4SDC100000004.544

Optimized implementation results:

namenthreadstypesizemedian
Series.rolling.sum4Python100000000.552
Series.rolling.sum4SDC100000000.053

The optimized implementation executes faster up to ~85 times than previous one and faster up to ~10 times than Python. There is no scalability due to prange isn't used at all because variable nfinite (number of finite values) is common for all threads.

@densmirn
densmirnforce-pushed the feature/series_rolling_sum_opt branch from 066cd4a to 178a4d9CompareFebruary 17, 2020 07:31
@densmirndensmirn changed the title Reimplement series.rolling.sum()Optimize series.rolling.sum()Feb 17, 2020
@densmirn
densmirnforce-pushed the feature/series_rolling_sum_opt branch from 178a4d9 to b2a4d9dCompareFebruary 17, 2020 12:15
output_arr = numpy.empty(length, dtype=float64)

chunks = get_chunks(length)
for i in prange(len(chunks)):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does it helped? Can't wait to know the result 😄

BTW you are not going to write all this monstrous code for every rolling function, don't you?

@AlexanderKalistratovAlexanderKalistratovFeb 17, 2020

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm expecting to see generic implementation for the most of series methods something like this:

windows= [WindowKind(window_size)]
foriinrange(1, len(chunks)):
windows.append(WindowKind(window_size))
foriinprange(len(chunks)):
chunk=chunks[i]
window=windows[i]
prelude_start=max(0, chunk.start-window_size)
prelude_stop=max(0, chunk.start)
forjinrange(interlude_start, interlude_stop):
window.add(data, j)
forjinrange(chunk.start, chunk.stop)
window.add(data, j)
result[j] =window.get_result()

This is a pseudocode. You need to think about exact details

@pep8speaks

pep8speaks commented Feb 18, 2020

Copy link
Copy Markdown

Hello @densmirn! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-02-20 05:35:37 UTC

@densmirndensmirn left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Current performance:

namenthreadstypesizemedian
Series.rolling.sum1Python2000001.219
Series.rolling.sum1SDC2000000.905
Series.rolling.sum4Python2000001.23
Series.rolling.sum4SDC2000000.409

Python 1 / SDC 4 = 2,98
The scalability was enabled.

@densmirn
densmirnforce-pushed the feature/series_rolling_sum_opt branch from 41896f4 to ac675e5CompareFebruary 19, 2020 16:38

@densmirndensmirn left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Current performance:

namenthreadstypesizemedian
Series.rolling.sum1Python8000003.903
Series.rolling.sum1SDC8000000.517
Series.rolling.sum4Python8000003.947
Series.rolling.sum4SDC8000000.254

Python 1 / SDC 1 = 7.549
Python 1 / SDC 4 = 15,366

Remeasured linear implementation b2a4d9d:

namenthreadstypesizemedian
Series.rolling.sum1Python8000004.01
Series.rolling.sum1SDC8000000.401

SDC_LINEAR 1 / SDC_PARALLEL 1 = 0.776
SDC_LINEAR 1 / SDC_PARALLEL 4 = 1.579

I think it's a victory.

return nfinite, result


def gen_sdc_pandas_series_rolling_impl(pop, put, init_result=numpy.nan):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please consider the following option:

@sdc_register_jitabledefresult_or_nan(nfinite, minp, result):
ifnfinite<minp:
returnnumpy.nanreturnresultdefgen_sdc_pandas_series_rolling_impl(pop, put, init_result=numpy.nan):
"""Generate series rolling methods implementations based on pop/put funcs"""defimpl(self):
win=self._windowminp=self._min_periodsinput_series=self._datainput_arr=input_series._datalength=len(input_arr)
output_arr=numpy.empty(length, dtype=float64)
chunks=parallel_chunks(length)
foriinprange(len(chunks)):
chunk=chunks[i]
nfinite=0result=init_resultprelude_start=max(0, chunk.start-win+1)
prelude_stop=min(chunk.start, prelude_start+win)
interlude_start=prelude_stopinterlude_stop=min(prelude_start+win, chunk.stop)
foridxinrange(prelude_start, prelude_stop):
value=input_arr[idx]
nfinite, result=put(value, nfinite, result)
foridxinrange(interlude_start, interlude_stop):
value=input_arr[idx]
nfinite, result=put(value, nfinite, result)
output_arr[idx] =result_or_nan(nfinite, minp, result)
foridxinrange(interlude_stop, chunk.stop):
put_value=input_arr[idx]
pop_value=input_arr[idx-win]
nfinite, result=put(put_value, nfinite, result)
nfinite, result=pop(pop_value, nfinite, result)
output_arr[idx] =result_or_nan(nfinite, minp, result)
returnpandas.Series(output_arr, input_series._index,
name=input_series._name)
returnimpl

It's not the most elegant one, but it could give us some performance (due to elimination of condition in loop and extra counter). If it doesn't, your solution is preferable.

Also, I've changed order of put and pop (firstly put, then pop). It shouldn't affect sum, but could be useful for min and max - if we have added new min/max - we don't need to recalculate result

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I didn't get visible result, but I like the code. So let me apply the patch.

@AlexanderKalistratov

Copy link
Copy Markdown
Collaborator

Also please keep in mind, that for some functions you need to keep more than one result (e.g. variance)

@AlexanderKalistratov
AlexanderKalistratov merged commit 632b554 into IntelPython:masterFeb 20, 2020
@densmirn
densmirn deleted the feature/series_rolling_sum_opt branch June 9, 2020 12:12
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@densmirn@pep8speaks@AlexanderKalistratov