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

Smoothed curve way off the actual line #1439

Closed
danijar opened this issue Sep 19, 2018 · 2 comments
Closed

Smoothed curve way off the actual line #1439

danijar opened this issue Sep 19, 2018 · 2 comments

Comments

@danijar
Copy link

danijar commented Sep 19, 2018

Without smoothing:
screenshot
Smoothing 0.9:
screenshot
I don't see any reason why the smoothed curve should be so far away from the actual data.

@TheGuywithTheHat
Copy link

TheGuywithTheHat commented Jul 12, 2019

I think this is because the current algorithm essentially sets each smoothed point to a weighted average of all original data points that come before it in the series. Relevant code here:

private resmoothDataset(dataset: Plottable.Dataset) {
let data = dataset.data();
const smoothingWeight = this.smoothingWeight;
// 1st-order IIR low-pass filter to attenuate the higher-
// frequency components of the time-series.
let last = data.length > 0 ? 0 : NaN;
let numAccum = 0;
const yValues = data.map((d, i) => this.yValueAccessor(d, i, dataset));
// See #786.
const isConstant = yValues.every((v) => v == yValues[0]);
data.forEach((d, i) => {
const nextVal = yValues[i];
if (isConstant || !Number.isFinite(nextVal)) {
d.smoothed = nextVal;
} else {
last = last * smoothingWeight + (1 - smoothingWeight) * nextVal;
numAccum++;
// The uncorrected moving average is biased towards the initial value.
// For example, if initialized with `0`, with smoothingWeight `s`, where
// every data point is `c`, after `t` steps the moving average is
// ```
// EMA = 0*s^(t) + c*(1 - s)*s^(t-1) + c*(1 - s)*s^(t-2) + ...
// = c*(1 - s^t)
// ```
// If initialized with `0`, dividing by (1 - s^t) is enough to debias
// the moving average. We count the number of finite data points and
// divide appropriately before storing the data.
let debiasWeight = 1;
if (smoothingWeight !== 1.0) {
debiasWeight = 1.0 - Math.pow(smoothingWeight, numAccum);
}
d.smoothed = last / debiasWeight;
}
});
}

Current policy on the smoothing algorithm, from #610 (comment):

It seems that every change in the smoothing algorithm results in new tradeoffs and biases, and new requests to change the algorithm. At this point it's not a priority for the team, so just filing an issue probably won't directly result in changes. However, the smoothing algorithm is actually a very simple piece of code to change. If you come up with a new algorithm, and demonstrate persuasively why it is an improvement over the current one (taking into account new biases and edge cases that it introduces), I'll happily merge it.

@gowthamkpr
Copy link

Closing the issue due to lack of recent activity. Please add additional comments and we can reopen the issue again. Thanks!

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

No branches or pull requests

4 participants