bug: median absolute deviation is always zero in case of odd number of samples

[pallosp]

The root cause is that aggFn expects a sorted list, while absoluteDeviations is not sorted.

export const getStatisticsSorted = (samples: number[]): Statistics => {
  ...
  const p50 = medianSorted(samples)
  return {
    mad: absoluteDeviation(samples, medianSorted, p50),
    ...
  }
}

const absoluteDeviation = (
  samples: number[],
  aggFn: (arr: number[]) => number | undefined,
  aggValue = aggFn(samples)
) => {
  const absoluteDeviations: number[] = []

  for (const sample of samples) {
    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
    absoluteDeviations.push(Math.abs(sample - aggValue!))
  }

  return aggFn(absoluteDeviations)
}

[jerome-benoit]

getStatisticsSorted() is called only on a sorted sample. I do not understand how the number of samples is related to the MAD.

[pallosp]

I'm talking about absoluteDeviations.

For example, when samples are [1, 2, 3]

1. p50 will be 2
2. absoluteDeviation will be called with the ([1, 2, 3], medianSorted, 2) params
3. absoluteDeviations will be populated with [1, 0, 1]
4. MAD will be medianSorted([1, 0, 1]), which returns the 0 instead of 1

[jerome-benoit]

absoluteDeviations() only call site location is in getStatisticsSorted(). And getStatisticsSorted() is always called on a sorted sample.

[pallosp]

getStatisticsSorted() calls absoluteDeviation(), which inside calls medianSorted() with an unsorted list

[jerome-benoit]

I see it now :)

[pallosp]

I sent a PR with the fix

[jerome-benoit]

fixed in #236