bugfix ft_surrogate

[martinwimpff]

Bugfix as discussed in #408.

[codecov]

Codecov Report

Merging #409 (d889c3a) into master (9cc8c68) will increase coverage by 1.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #409      +/-   ##
==========================================
+ Coverage   83.03%   84.20%   +1.17%     
==========================================
  Files          55       46       -9     
  Lines        3838     3616     -222     
==========================================
- Hits         3187     3045     -142     
+ Misses        651      571      -80     

[cedricrommel]

I just changed the linting in a long line, but everything LGTM :) Thanks a lot @martinwimpff !
I'll carry some tests regarding the comment above and let you know.

[bruAristimunha]

Nice @martinwimpff and @cedricrommel,

@martinwimpff, can you update the what's new file?

[martinwimpff]

@cedricrommel I had the same thought but I think this version makes more "physical" sense: if you interprete the phase perturbation as a simple time shift in the time domain, the perturbation should be the same across all channels. I would guess that this would not really affect the performance but I am looking forward to your test results ;)

[cedricrommel]

So, results are that if we sample phase perturbations independently for each channel we break cross-channels correlations, which are instrumental in tasks such as BCI. So we see on BCI IV 2a for example that augmenting as you were doing @martinwimpff (FTSurrogate) is way better than doing it with fully random phases (FTSurrogateIND):

However, for tasks where cross-channel correlations don't matter that much like sleep staging, we obtain slightly better improvements if we randomize as much as possible (i.e. independently for each channel):

So I've added an argument to control this and set the default to do it your way @martinwimpff since it is safer.

Thanks a lot for the bug fix and very insightful discussion !! :)

If the modifications I've made look good to you @martinwimpff and @bruAristimunha , I'll merge the PR as soon as it passes the tests!

[bruAristimunha]

looks wonderful to me :)

[robintibor]

seems good so is this ready to merge?

[cedricrommel]

Yes! Le jeu. 29 sept. 2022 à 16:11, robintibor ***@***.***> a écrit :

…

seems good so is this ready to merge? — Reply to this email directly, view it on GitHub <#409 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AECZTBBT4BVJE3JBAJF34LLWAWPQJANCNFSM6AAAAAAQTBKBQ4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[cedricrommel]

Thank you guys!! 🙏