[ENH] - Add DFA to a new aperiodic module #167
Conversation
|
@rdgao : I refactored a bit to try and match NDSP format, and left a couple quick questions in a review. I want to sanity check I'm not missing anything before I go through with a couple other updates. Whenever you get a chance (no huge rush) throw an eye on the comments & sign off if things looks good. I'm happy to make the changes, if nothing looks weird. Only other open question is naming. 'fractal' or 'scale-free' feels a little broad, as many other measures relate to those properties. I don't want to call everything DFA, as it isn't all DFA. Is 'fluctuation analysis' an at all reasonable catch-all term for the measures here? Can we call this file 'fluctuations.py'? |
There was a problem hiding this comment.
I'm not sure about the demean thing, but everything else should be fine.
As for the name, I think fluctuations is a little too non-specific. In some sense, everything we do is looking at fluctuations, and this particular word is not usually used in that specific context. Could call it nonlinear? But yeah it really depends on what might go into that folder in the future. Could have just DFA as its own thing, or a more inclusive folder.
|
Okay @rdgao - I did the updates from the comments here. One convo is left open, if you want to throw an eye and make sure I didn't mess it up in the updates. Other than potentially changing the name of the file, this should be ready to merge, I think, if everything looks good to you. |
|
Code looks okay. I think the test should run it on a simulated white noise signal and get back what we expect? |
TomDonoghue
changed the title
| raise ValueError('Fluctuation method not understood.') | ||
|
|
||
| # Calculate the relationship between between fluctuations & time scales | ||
| exp = np.polyfit(np.log10(t_scales), np.log10(fluctuations), deg=1)[0] |
There was a problem hiding this comment.
So as a first step, I think fitting a line to the fluctuations in log-log land is fine. Looking down the road though, EEG signals have a DFA plot which is piecewise linear, at least empirically according to this work. Perhaps this is something we could add at a later point and wouldn't require too much extra work I think.
There was a problem hiding this comment.
Yeh, I think this is a good point / idea - but is more of an extension for future work, that doesn't need to go into this PR necessarily. As I understand it, what we have here is "standard" DFA, which we can always build on later.
|
I made some mild updates, based on the reviews - fixing the math error noted by Eric, updating tests to be more specific, as suggested by Richard, and updating the approach the window length issue brought up by Ryan (go team!). I think this should be good to go. @elybrand - maybe you want to take a quick look at my updates and how these functions are tested now? @ryanhammonds - everything new look good to you? |
|
Okay, all looks good here! Thanks for the code @rdgao, and the reviews @elybrand & @ryanhammonds! Merging in now to officially start our aperiodic module! |
|
wooo! |
Add Detrended Fluctuation Analaysis.
Edit from Tom: