Auto-estimate margins based on freq_min/f0+Q for filters #4227
Conversation
|
I think this is a great feature, but I already told @samuelgarcia that maybe we should at very least warn (prevent ?) users that will use very low frequencies for filters, trying to get LFP and/or low pass versions of the data. If the automatic margin starts to be really large (maybe larger than the default chunk size of 1s), then we should warn users that they are doing something clearly suboptimal. |
Sub-optimal but somehow needed! Maybe we can warn if the margin is greater than the |
|
Yes, I've read the PR, and indeed, at least a warning that the theoretical margin should be XX, and that it has be clipped to max_margin_s. The thing is that if users really want to do that, then the warning message could also advise them to increase chunk_duration in the default_job_kwargs, to reduce IO overloads |
|
We democratically decided to remove the max_margin kwargs and instead make a mechanism that raise an error if the end-user try to filter in LFP band and then point to a link to a clear and didactic and really exemplar tutorial about chunking and its bad effect on low freqs. |
|
This is ready for final review. here's a new doc page explaining the issue: https://spikeinterface--4227.org.readthedocs.build/en/4227/how_to/extract_lfps.html |
|
Hello, amazing! You could set this doc up as a jupyter notebook that gets run in the CI (e.g. https://github.com/SpikeInterface/spikeinterface/tree/main/examples/tutorials/forhowto) Then we don't have to do all the .py -> .rst conversion annoying stuff. |
Done! Also moved the |
|
@chrishalcrow needed some juggling to make it work with RTD RAM (the script used to save everything in memory), but it works now! Let me know if you have more comments: https://spikeinterface--4227.org.readthedocs.build/en/4227/forhowto/plot_extract_lfps.html |
| # a "margin" (extra samples at the edges) to avoid edge artifacts when filtering. Let's demonstrate | ||
| # this by saving the filtered data with different chunking strategies. | ||
| # | ||
| # **This error is to inform the user that extra care should be used when dealing with LFP signals!** |
There was a problem hiding this comment.
The start of this section is a bit confusing. This line refers to the error from the previous section -- but since we're in a new section I think the reader will be confused. Maybe move this statement to just after the error?
| _ = sw.plot_traces(recording_filt, time_range=[20, 21]) | ||
|
|
||
| ############################################################################## | ||
| # A warning tells us that what we are doing is not optimized, since in order to get the requested traces |
There was a problem hiding this comment.
There's no warning in the output now, sorry, probably something to do with changing how it's rendered?
There was a problem hiding this comment.
I do see the warning locally, but not on the build docs...not sure why! Should I just remove it?
| ftype="butter", | ||
| filter_mode="sos", | ||
| margin_ms=5.0, | ||
| margin_ms=5, |
There was a problem hiding this comment.
| margin_ms=5, | |
| margin_ms=5.0, |
4 participants
@samuelgarcia let's discuss!