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 |
| # when not done carefully, especially when data is processed in chunks (for memory efficiency). | ||
| # | ||
| # This tutorial demonstrates: | ||
| # 1. How to generate simulated LFP data |
There was a problem hiding this comment.
I think you need a space to render this as a list
| # 1. How to generate simulated LFP data | |
| # 1. How to generate simulated LFP data |
| # **Why does this fail?** | ||
| # | ||
| # The error occurs because by default in SpikeInterface when highpass filtering below 100 Hz. | ||
| # Filters with very low cutoff frequencies have long impulse responses, which require larger margins to avoid edge artifacts between chunks. |
There was a problem hiding this comment.
I think we need to explain what a margin and a chunk is more clearly before this?
If I were writing it, I'd have a first section called "1. Chunks and margins" and explain what chunking is, why we do it, and what a margin is. Or put in more explanation here.
There was a problem hiding this comment.
Oh, I've read more now... so I would put Section 3 earlier in the tutorial. And put in a longer description of a chunk and margin.
| ax.legend() | ||
|
|
||
| # %% [markdown] | ||
| # For smaller chunk sizes, these artifact will happen more often. In addition, the margin "overhead" will make processing slower. |
There was a problem hiding this comment.
This comes straight after the plot. Can you explain what we see in the plot. Something like:
From the plot, we can see that there is a very small error when the margin size is large (green), a larger error when the margin is smaller (organge) and a large error when the margin is small (blue). So we need large margins (compared to the chunk size) if we want accurate filtered.
For smaller...
| continue | ||
| diff = recording_optimal - recording_chunk | ||
| traces_diff = diff.get_traces(start_frame=start_frame, end_frame=end_frame) | ||
| ax.plot(traces_diff, label=recording_key) |
There was a problem hiding this comment.
Can you make the axes in seconds, please? Then it's much easier to see where 30s is.
| sw.plot_traces(recording_filt, time_range=[20, 21]) | ||
|
|
||
| # %% [markdown] | ||
| # A warning tells us that what we are doing is not optimized, since in order to get the requested traces the marging "overhead" is very large. |
There was a problem hiding this comment.
| # A warning tells us that what we are doing is not optimized, since in order to get the requested traces the marging "overhead" is very large. | |
| # A warning tells us that what we are doing is not optimized, since in order to get the requested traces the margin-ing "overhead" is very large. |
? or just "margin"
|
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. |
4 participants
@samuelgarcia let's discuss!