Fix downsample_median returning the mean for even periods - #853
Fix downsample_median returning the mean for even periods#853adityasingh2400 wants to merge 4 commits into
Conversation
downsample_median short circuits to downsample_mean whenever the period is even, so perform_downsampling with AggOperations::MEDIAN is bit for bit identical to AggOperations::MEAN for every even period. That defeats the outlier rejection which is the reason to pick a median filter in the first place. A single large spike inside a window is passed straight through, divided by the period, instead of being discarded. The library already contains the right answer. RollingMedian in rolling_filter.h, which backs perform_rolling_filter, averages the two middle values for even window lengths. The two median paths disagreed with each other on identical input. This makes downsample_median use the same convention: sort first, then average the two middle values when the length is even. For a period of 2 the correct even median happens to equal the mean, so behaviour there is unchanged. Odd periods are unchanged as well.
|
The implementation now matches Please add an automated downsampling case for periods 2, 3, and 4 with an outlier, asserting that period 4 returns the mean of the two sorted middle values rather than the full-window mean. Since this changes established output for every even period greater than 2, please also mention it in the release notes if numerical behavior changes are normally documented. |
Covers periods 2, 3 and 4 over a window with an outlier, and asserts period 4 returns the mean of the two sorted middle values rather than the full-window mean.
|
Added It runs periods 2, 3 and 4 over a window with an outlier. Period 4 asserts On release notes, I could not find a changelog file in the repo. If numerical changes get noted somewhere outside git, point me at it and I will add an entry. |
|
Follow-up: I had only wired the new test into Both workflow files still parse. |
2 participants
downsample_medianinsrc/data_handler/inc/downsample_operators.hshort circuits todownsample_meanwhenever the period is even.perform_downsamplingwithAggOperations::MEDIANis therefore bit for bit identical toAggOperations::MEANfor every even period, which defeats the outlier rejection that is the only reason to choose a median filter. A single large spike inside a window passes straight through, divided by the period, instead of being discarded.The library already contains the correct answer.
RollingMedianinrolling_filter.h, which backsperform_rolling_filter, averages the two middle values for even window lengths. So BrainFlow has two median implementations that disagree with each other on identical input. This makesdownsample_medianfollow the same convention: sort first, then average the two middle values when the length is even.Two things worth stating precisely.
perform_rolling_filteris not affected, onlyperform_downsampling. And at period 2 the correct even median is by definition the mean of the two values, so behaviour there is unchanged. Odd periods are unchanged.Verified by building
libDataHandler.dylibon macOS arm64. Input is a quiet baseline around 10.0 with one 1000.0 spike per window.perform_downsamplingwith MEDIAN, window 0:Spike leakage above the 10.0 baseline, which is the number that actually matters for a median filter:
Before the fix MEDIAN leaked the spike identically to MEAN. After, it rejects it.
Cross check of the two median paths on identical data,
perform_downsamplingMEDIAN againstperform_rolling_filterMEDIAN aligned to the same windows: 10 of 10 windows mismatched at period 4 and 5 of 5 at period 8 before the fix, with 257.25 against 9.75, a 26x disagreement. 0 of 10 and 0 of 5 mismatched after.All 9 signal processing examples that CI runs pass on this branch, and
clang-formatreports no changes on the touched file.Disclosure: this change was prepared with AI assistance. I have reviewed and tested it.