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

Feature "Moving Window Cross-Spectrum" #1719

Open
wants to merge 15 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@ThomasLecocq
Contributor

ThomasLecocq commented Mar 13, 2017

What does this PR do?

Adding the Moving Window Cross-Spectral method. This method allows comparing two signals in the frequency domain (cross-spectrum, or cepstrum) and returns a table of time delays (offsets) vs time for each window.
See current doc here:
http://msnoise.org/doc/master/core.html#moving-window-cross-spectral-method

PR Checklist

  • it is linked to #841 and branched from #1716

  • All tests still pass.

  • Any new features or fixed regressions are be covered via new tests.

  • Any new or changed features have are fully documented.

  • Significant changes have been added to CHANGELOG.txt .

  • I should provide an example with some figures.

This PR does look ugly because I actually branched it from #1716 , didn't really know how to do it otherwise as I need the linear_regression in this PR.

ThomasLecocq added some commits Mar 13, 2017

@barsch

This comment has been minimized.

Show comment
Hide comment
@barsch

barsch Mar 13, 2017

Member

weight

weight

@ThomasLecocq

This comment has been minimized.

Show comment
Hide comment
@ThomasLecocq

ThomasLecocq Mar 13, 2017

Contributor

raaaah, indeed !

Contributor

ThomasLecocq commented on 682042e Mar 13, 2017

raaaah, indeed !

ThomasLecocq added some commits Mar 13, 2017

@ThomasLecocq ThomasLecocq changed the title from Feature mwcs to Feature "Moving Window Cross-Spectral" Mar 13, 2017

@ThomasLecocq ThomasLecocq changed the title from Feature "Moving Window Cross-Spectral" to Feature "Moving Window Cross-Spectrum" Mar 13, 2017

@ThomasLecocq ThomasLecocq added this to the 1.1.0 milestone Mar 13, 2017

ThomasLecocq added some commits Mar 14, 2017

@ThomasLecocq

This comment has been minimized.

Show comment
Hide comment
@ThomasLecocq

ThomasLecocq Mar 14, 2017

Contributor

this fails on Travis while it's OK on appveyor, it is linked to scipy' next_fast_len being available or not. In Appveyor scipy is not fixed, so it uses 0.18.1, while in Travis it seems fixed. The results of my test are different with obspy's next_pow_2 or next_fast_len ... not good.. have to find a way to check for both (and ensure the results are "similar" enough to show the method is stable).

And it's kind of scary that the linear regression is so different for different n submitted to the FFT.

Contributor

ThomasLecocq commented Mar 14, 2017

this fails on Travis while it's OK on appveyor, it is linked to scipy' next_fast_len being available or not. In Appveyor scipy is not fixed, so it uses 0.18.1, while in Travis it seems fixed. The results of my test are different with obspy's next_pow_2 or next_fast_len ... not good.. have to find a way to check for both (and ensure the results are "similar" enough to show the method is stable).

And it's kind of scary that the linear regression is so different for different n submitted to the FFT.

@ThomasLecocq

This comment has been minimized.

Show comment
Hide comment
@ThomasLecocq

ThomasLecocq Mar 24, 2017

Contributor

@barsch @megies @krischer

The tests fail depending on the scipy version available, because the results of the MWCS are slightly different when using nextpow2 or next_fast_len ? Should this function be defined with nextpow2 by default but allow for some sort of callback in case one wants to use next_fast_len (scipy >= 0.18) ?

Contributor

ThomasLecocq commented Mar 24, 2017

@barsch @megies @krischer

The tests fail depending on the scipy version available, because the results of the MWCS are slightly different when using nextpow2 or next_fast_len ? Should this function be defined with nextpow2 by default but allow for some sort of callback in case one wants to use next_fast_len (scipy >= 0.18) ?

@ThomasLecocq ThomasLecocq modified the milestones: 1.2.0, 1.1.0 Mar 27, 2017

@megies megies referenced this pull request Mar 30, 2017

Closed

generalized linear_regression #1716

6 of 6 tasks complete
@krischer

This comment has been minimized.

Show comment
Hide comment
@krischer

krischer Apr 6, 2017

Member

Can you up the test tolerance to have it test similar enough on different scipy versions?

Member

krischer commented Apr 6, 2017

Can you up the test tolerance to have it test similar enough on different scipy versions?

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