BBN-Q / Qlab.jl

Generic lab tools in Julia
Other
12 stars 7 forks source link

add interpolation option for Savitzky Golay filtering boundary condition handling #58

Closed henry2004y closed 4 years ago

henry2004y commented 4 years ago

Hi,

I find that the savitzkyGolay() filter is not working properly on the boundary points.

Take this simple example, with first order polynomial and window size 5:

x=[1,2,3,4,5,6,7,8,9,10]
savitzkyGolay(x,5,1)

where the correct result should be x, but the corresponding function in Julia returns:

1.6
2.2
3.0
4.0
5.0
6.0
7.0
8.0
8.8
9.4

Hope there's a quick fix! Thanks!

caryan commented 4 years ago

I'm not sure there is a "correct" way to handle boundary conditions. We currently pad the signal with the endpoints here which is the same behaviour as mode=nearest in scipy.signal.savgol_filter. It seems what @henry2004y is looking for is an interpolation option for extension.

henry2004y commented 4 years ago

I see. I agree that there's no correct way of handling this, and maybe in the end it doesn't matter that much to the final results. However, since Matlab and Python both implement interpolation as the default (or maybe only) option, I really hope it would be consistent also in Julia.

Thanks for the response!

caryan commented 4 years ago

Although I've wanted to clean up this implementation for a while so I could easily add the interpolation options. Changing issue title appropriately.

caryan commented 4 years ago

@henry2004y the PR ^^ should give you the behavior you expect for the boundaries.

henry2004y commented 4 years ago

Just a minor reminder about the typo of the function name: Savitsky -> Savitzky

matthewware commented 4 years ago

Thanks @henry2004y! Should be in master now.

caryan commented 4 years ago

Oof! Thanks @henry2004y.