-
Notifications
You must be signed in to change notification settings - Fork 61
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
[DOC] - Add tutorials for aperiodic sub-module #231
Conversation
Hi @TomDonoghue, these tutorials should be ready for review.
|
@ryanhammonds - awesome, thanks for your work on this one! In terms of code updates, the autocorr lag length is a good catch! The threshold for IRASA is interesting, I'm not sure if it might have some quirks in some cases, but given it defaults to off, and helps in some cases, then I think it's a cool / useful things to keep. For the tutorials, I went through and did direct edits, and added a bit of stuff. The only major change is the autocorr, in that I wasn't really clear where the phase comparisons were going, so I switched that up (if I seem to have missed something there, let me know, and we can perhaps add some stuff back). When you get a chance, can you review the updates to the tutorials? If they look good to you, then I think this one is done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @TomDonoghue, I found a few typos but other than that it looks good.
The phase comparison tried to show how the phase changes as a result of lagging the signal. The second removed plot was representative of the correlation between lags at difference points on the first plot. No worries about removing it though. If it wasn't clear it shouldn't go in.
Thanks for the review @ryanhammonds! I've updated the files, and I think this one is good to go now. Also: ahhh, I think I've just tracked the phase delay thing of the autocorr example - showing different delays of the same signal. I think that's an interesting demo, that would perhaps just need a bit more explanation. If you ever feel like integrating it back in, feel free to update & PR! |
This is a placeholder / WIP PR for adding tutorials for the aperiodic module.
@ryanhammonds : can you have a first go at developing these tutorials? They can start as pretty simple, showing code application, and then I can check and fill in any details later.