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

Filter frequencies << sampling frequency are unstable #1028

Closed
wants to merge 2 commits into from

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Apr 27, 2015

Hi,

I just found a possible question about butterworth-bandpass filtering. I download a seismogram from IRIS and try to filter it with a butterworth-bandpass filter from 0.02hz to 0.2hz, but found the result is incorrect with some extremely large number after filtering. I guess it might be caused by some kind of unstability of filtering algorithm. Here is my code:

from obspy.fdsn import Client
from obspy import UTCDateTime
from obspy.taup.taup import getTravelTimes
from obspy.core import AttribDict
client = Client('iris')
starttime = UTCDateTime('2014-08-24T10:20:44.0600')
p_arrival_time = 376.945875099  # p phase arrival time
st = client.get_waveforms(network='AK', station='BPAW', location='*', channel='BHZ', 
                          starttime=starttime+p_arrival_time-60, endtime=starttime+p_arrival_time+180,
                          attach_response=True)
st.remove_response(output='disp')  # remove the instrument response
st.detrend(type='demean')  # remove the mean
st.detrend(type='linear')  # remove the linear trend
st.taper(type='hann', max_percentage=0.05)  # taper
# st.resample(20)  # the decreasing samping rate can make filtering work well
st.filter('bandpass',freqmin=0.02, freqmax=0.2, zerophase=True)  # filter
st.write('tmp.sac',format='sac')

BTW, when I decrease the sampling rate from original 50hz to 20hz, the filtering will be good. I wonder whether it might also be related to sampling rate.

I also find a similar question about scipy:
http://stackoverflow.com/questions/21862777/bandpass-butterworth-filter-frequencies-in-scipy
Since the filtering in obspy is actually based on scipy, they possibly point to one same question....

Thx

@QuLogic
Copy link
Member

QuLogic commented Apr 25, 2015

Based on this answer, I think we should a) add a warning if the filtering frequencies are very small compared to the sampling rate, and b) switch to sosfilt once a new scipy is available.

@QuLogic QuLogic added the .signal issues related to our signal processing functionalities label Apr 25, 2015
@QuLogic QuLogic changed the title A question about butterworth-bandpass filtering Filter frequencies << sampling frequency are unstable Apr 25, 2015
@krischer
Copy link
Member

I think we should backport the second-order filter sections from scipy. This has been an annoying issue for a long time and we cannot simply depend on scipy 0.16 once available.

That should be fairly quick to do and incredibly useful.

@QuLogic
Copy link
Member

QuLogic commented Apr 27, 2015

It looks like we'd need sosfilt, zpk2sos and all filters need to support output='zpk'. I'm not sure when zpk output was added? Also, zpk2sos is pretty long...

The second-order sections form for filters are much more stable than the
form using tranfer functions.
@QuLogic
Copy link
Member

QuLogic commented Apr 27, 2015

Well, there you go...
sosfilt
But you'd need to check whether zpk form is supported all the way back in scipy 0.7.2.

@krischer
Copy link
Member

Nice :-) The results are quite spectacular. Its possible to choose much higher order filters than before. Eventually the new filters still become unstable but that is to be expected and it is now MUCH better.

Left is before, right is after. The title are the settings for the bandpass filter.

bandpass_filters

@megies
Copy link
Member

megies commented Apr 27, 2015

@megies
Copy link
Member

megies commented Apr 27, 2015

Two questions..

  • the idea is that this is considered a bug fix and goes into 0.10.x?
  • should we make it a compatibility layer, trying to use scipy (for >= 0.16, for sake of upstream bug fixes/improvements) and only using our copied code for older scipy?

@krischer
Copy link
Member

No to both in my opinion.

This is not a bugfix but rather a different algorithm.

I would also tend to avoid having a compatibility layer and rather just merge upstream fixes into our backport if we notice them. Otherwise the danger is that the same version of ObsPy with two different scipy yields very different results.

@ThomasLecocq
Copy link
Contributor

👍 wooooow ! I can finally remove the low+high pass two lines in msnoise :)

@megies
Copy link
Member

megies commented Apr 27, 2015

This is not a bugfix but rather a different algorithm.

Then the base branch would have to be changed..

rather just merge upstream fixes into our backport if we notice them.

means a maintenance overhead..

the same version of ObsPy with two different scipy yields very different results.

That's one way to look at it.. you could also say, having a newer scipy would immediately bring it's benefits without having to wait for a new obspy release. Also, that statement would apply to any of our dependencies.

@QuLogic
Copy link
Member

QuLogic commented Apr 27, 2015

the idea is that this is considered a bug fix and goes into 0.10.x?

Sorry, I snuck this one onto the releases target... 😛

should we make it a compatibility layer, trying to use scipy (for >= 0.16, for sake of upstream bug fixes/improvements) and only using our copied code for older scipy?

It already does so. zpk2sos and such are really long; I don't feel like maintaining them unless really necessary.

@krischer
Copy link
Member

@megies @QuLogic Alright...point taken :-) Let's have a compatibility layer.

But this should still not got into releases I think as the change is fairly serious and will affect everyone.

@krischer
Copy link
Member

This PR works fine with scipy 0.9.0: http://tests.obspy.org/24261/

Assuming #1031 is ok with everyone and we don't merge it in releases it should be good to go.

@QuLogic QuLogic self-assigned this Apr 27, 2015
@QuLogic QuLogic added this to the 0.11.0 milestone Apr 27, 2015
@QuLogic QuLogic deleted the signal-sosfilt branch April 27, 2015 21:21
@XStargate
Copy link
Author

After testing the backport sosfilt from scipy 0.16.0, the filtering waveform is the same with waveform filtered by sac even for some extremely small frequency bandpass (e.g. 0.02-0.2hz here). The first plot is filtered by sac and the second one is filtered by obspy sosfilt

filtering

The difference between two filtering is as small as 1e-9 which can be ignored.

difference

@megies
Copy link
Member

megies commented Apr 30, 2015

Interesting.. thanks for the feedback.

@megies megies added the enhancement feature request label Apr 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature request .signal issues related to our signal processing functionalities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants