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

correct 1, 5, 15 minute smoothing factors #4

Closed
wants to merge 1 commit into from
Closed

correct 1, 5, 15 minute smoothing factors #4

wants to merge 1 commit into from

Conversation

jdmaturen
Copy link

MeterMetric's EWMA function is correct, presuming FACTOR means α, the smoothing factor. However the factors themselves are miscalculated.

The one minute factor is actually 1-α, aka the EXP_R. Five minute and 15 minute factors are calculated in the right direction but only incidentally. Had they been formulated the same way as the one minute EXP_R, i.e. exp(-5/60/5) and exp(-5/60/15), they would be damping even less than the one minute factor. Also, though you can approximate the 5 and 15 minute alphas by dividing the 1 minute alpha, the result is off by a couple percent.

Attached patch to fix the factors. You may also want to rename FACTOR to ALPHA. Note that this dramatically changes the damping effect [the curve is approximately 11x slower]; users on the new code will see quite different behavior for the same input.

The kernel load average EWMA code stores 1, 5, 15 minute 'factors' as hard coded fixed point 11 bit EXP_Rs instead of alphas for the sake of speed and size. The load average macro itself is written in terms of EXP_R instead of alpha [and is the basis of my initial suggestion on twitter]. Perhaps this is where the error originated.

Hopefully I'm not way off in left field here, and thanks for the great project.

@codahale
Copy link
Contributor

I ended up extracting the EWMA code for better testability in acf5206, which incorporates this change.

Thank you very much for this.

@codahale codahale closed this Apr 10, 2011
@codahale
Copy link
Contributor

(acf5206, that is)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants