Skip to content
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.

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

incorrect calculation of centered moving averages for even length series #14425

Closed
simonm3 opened this issue Oct 14, 2016 · 7 comments
Closed
Labels
API Design Duplicate Report Duplicate issue or pull request Needs Discussion Requires discussion from core team before further action Numeric Operations Arithmetic, Comparison, and Logical operations

Comments

@simonm3
Copy link

simonm3 commented Oct 14, 2016

For even numbered periods the centered moving average is calculated incorrectly. Suppose the period length is 5. Then the center of 5 periods is 3. However if the period length is 4 then the center of the period is 2.5. The value at index 3 should be the average of the values at 2.5 and 3.5. Pandas is showing the 2.5 value at 3 which is incorrect.

EXAMPLE:
a=pd.Series(range(1,6), index=range(1,6))
a.rolling(4, center=True).mean()

1 NaN
2 NaN
3 2.5
4 3.5
5 NaN

@jreback
Copy link
Contributor

jreback commented Oct 14, 2016

center is a very funny beast. The computation is done then the results are shifted. To be honest we should just remove this.

This has been this way for quite a long time.

@jreback jreback added API Design Numeric Operations Arithmetic, Comparison, and Logical operations Needs Discussion Requires discussion from core team before further action labels Oct 14, 2016
@jreback
Copy link
Contributor

jreback commented Oct 14, 2016

actually this is a duplicate of #2953. has been open a long time.

@jreback jreback closed this as completed Oct 14, 2016
@jreback jreback added the Duplicate Report Duplicate issue or pull request label Oct 14, 2016
@jreback jreback added this to the No action milestone Oct 14, 2016
@simonm3
Copy link
Author

simonm3 commented Oct 14, 2016

This is not the same issue. The one you refer to is apparently affecting
only the edge values. I am saying all values are shifted half an index so
wrong.

Also it is easy to fix. It is just an test for length even and then
averaging each consecutive pair. And the suggestion it might be slower is
bizarre. It is slower to calculate 2+2 =4 than to just set it to 1 but that
would be incorrect.

On 14 Oct 2016 9:46 p.m., "Jeff Reback" notifications@github.com wrote:

Closed #14425.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@simonm3
Copy link
Author

simonm3 commented Oct 14, 2016

And SPSS, sas, and minitab all calculate centred moving averages correctly.

On 14 Oct 2016 11:25 p.m., "simon mackenzie" simonm3@gmail.com wrote:

This is not the same issue. The one you refer to is apparently affecting
only the edge values. I am saying all values are shifted half an index so
wrong.

Also it is easy to fix. It is just an test for length even and then
averaging each consecutive pair. And the suggestion it might be slower is
bizarre. It is slower to calculate 2+2 =4 than to just set it to 1 but that
would be incorrect.

On 14 Oct 2016 9:46 p.m., "Jeff Reback" notifications@github.com wrote:

Closed #14425.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@chris-b1
Copy link
Contributor

@simonm3 - there's a lot going on in that issue, but changing the centered mean was part of it - this reference, which I think is the approach you expect?
http://www.itl.nist.gov/div898/handbook/pmc/section4/pmc422.htm

I'm guessing centered mean isn't used that often, but if there is a standard approach for double smoothing an even window, and you would like to submit a PR to support that, I think that would be great.

@chris-b1
Copy link
Contributor

I guess you can also fairly easily solve it like this?

s.rolling(4, center=True).mean().rolling(2).mean()

@pkch
Copy link

pkch commented Jul 17, 2018

It would have to be:

s.rolling(4, center=True).mean().rolling(2).mean().shift(-1)

The current calculation of s.rolling(k, center=True) for an even k is clearly wrong, since it's not centered.

Also, IMHO, the issue linked to this as a duplicate really is not about even-sized centered windows, it's about some more complex issues.

Given how easy it is to fix this, perhaps it's worth reopening it, so someone can fix it with a small PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Duplicate Report Duplicate issue or pull request Needs Discussion Requires discussion from core team before further action Numeric Operations Arithmetic, Comparison, and Logical operations
Projects
None yet
Development

No branches or pull requests

4 participants