High level function for drift : correct_motion()#1300
High level function for drift : correct_motion()#1300alejoe91 merged 38 commits intoSpikeInterface:mainfrom
Conversation
|
There is a typo in the function name. Plus, I think it would make sense to have a default peak number selected, as we discussed, that would depend on the duration of the recording. Do you really want the default params for select peaks to be None? |
|
Salut Pierre. I am also thinkink of having some "preset" option. |
alejoe91
added
sortingcomponents
yger
changed the title
samuelgarcia
changed the title
|
|
||
| from spikeinterface.core import get_noise_levels, fix_job_kwargs | ||
| from spikeinterface.core.core_tools import SIJsonEncoder | ||
|
|
| margin_um=0.0, | ||
| win_shape="gaussian", | ||
| win_step_um=50.0, | ||
| win_sigma_um=150.0, |
There was a problem hiding this comment.
this seems like a lot of very narrow windows, I would expect very noisy results in real data. I usually use more like win_step_um=400.0, win_sigma_um=200.0 (I think that the window sigma should be divided by 2 in the gaussian case, since gaussian kernels really extend out to 2 or 3 std devs, so this would match the behavior of rect windows more closely -- in that case, we could set win_sigma_um=400 which "looks nicer". might also be worth changing sigma->width since sigma comes from the gaussian kernel, and this wouldn't be sigma anymore in that case)
There was a problem hiding this comment.
^ what I have written would currently lead to an error because window sigma cannot be < window step, but I think these params are the way to go and that that restriction should be removed -- been meaning to make a PR about that
There was a problem hiding this comment.
For win_step_um should not change the results lead to some kind of coninuum in space that avoid linear interpolation. I can be win_sigma_um=200 but you are suggesting to put 100 no ?
I am not sure to follow you.
There was a problem hiding this comment.
sorry, I was confusing!
yeah, I usually use win_sigma_um=200 or 250 in real data
and then the other thing I was saying, which is a separate issue, is that I think it could be nice to reparameterize the gaussian window size by a parameter which is 2 * win_sigma_um, which would make the width of the windows match the rect windows more closely
| peak_locations, | ||
| times, | ||
| motion, | ||
| temporal_bins, |
There was a problem hiding this comment.
it would be useful to clarify whether these are bin edges or bin centers
There was a problem hiding this comment.
I think it would be nice if estimate_motion returned an object so that this function could take fewer arguments, and to avoid replicating the logic about temporal bins and motion interpolation in several places (here in peaks and in traces and anywhere else a user may want to get displacement at a particular depth and time)
maybe a class like this one? https://github.com/evarol/dredge/blob/main/dredge-python/dredge/motion_util.py#L22
There was a problem hiding this comment.
This is bin center this is in the docstring no ?
I am agree for the object but this will be in the next round of modification that we will to togoether no ?
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
5 participants
This implement a high level function to estimate and interpolate dirft.