Dredge lfp and dredge ap #3062
Dredge lfp and dredge ap #3062
Conversation
src/spikeinterface/sortingcomponents/motion/tests/test_motion_estimation.py
Outdated
Show resolved
Hide resolved
Move method into separated files. Change, refactor (and rename some) kwargs from common to specific methos. Add dredge_lfp class.
|
@cwindolf the last commit is a quite big refactoring in estimate motion to integrate the 'dredge_lfp' method. |
|
@cwindolf I could not resists in porting also the dredge_ap and make a class. |
| mincorr_percentile_nneighbs=None, | ||
| # raster arguments | ||
| amp_scale_fn=None, ## @Charlie this one is not used anymore | ||
| post_transform=np.log1p, ###@this one is directly transimited to weight_correlation_matrix() and so get_wieiith() |
There was a problem hiding this comment.
actually we can probably not even transmit it. we can apply the function to the threshold in advance before passing it to weight_correlation_matrix to reduce the number of params. I can do this in my PR.
There was a problem hiding this comment.
ok. I left the code like this for the moment.
| rigid=False, | ||
| # nonrigid window construction arguments | ||
| win_shape="gaussian", | ||
| win_step_um=400, |
There was a problem hiding this comment.
I still think that these defaults are the best in typical NP1 data :)
| thomas_kw, xcorr_kw, raster_kw, weights_kw | ||
| These dictionaries allow setting parameters for fine control over the registration | ||
| device : str or torch.device | ||
| What torch device to run on? E.g., "cpu" or "cuda" or "cuda:1". |
There was a problem hiding this comment.
I'll add more documentation of the other parameters in my PR.
| smooth_kernel = smooth_kernel[:, None] | ||
| motion_clean = scipy.signal.fftconvolve(motion_clean, smooth_kernel, mode="same", axes=0) | ||
|
|
||
| return motion_clean |
There was a problem hiding this comment.
Where is this function used? Should it be an option at the end of estimate_motion, extending the signature with motion_cleaner_kwargs as a dict, and if not None, it is launched?
There was a problem hiding this comment.
I think it's not used currently, but it's an option. Let's discuss this later
There was a problem hiding this comment.
It is now separated from the main estimate. I think this is better. and honestly nobody used it.
| return motion_histogram, temporal_bin_edges, spatial_bin_edges | ||
|
|
||
|
|
||
| def make_3d_motion_histograms( |
There was a problem hiding this comment.
This is a bit of a pity that make_2d_motion_histogram and makd_3d_motion_histogram are not the same function, one being a particular case of the other. Because conceptually, they should. We could also propose some smoothing for 3d histogram over time, amplitudes and depth. What do you think? One single function could ease the code maintenance
There was a problem hiding this comment.
let's make it in a follow up PR
…ty to iterative_template
|
| @@ -13,7 +13,7 @@ def test_estimate_and_correct_motion(create_cache_folder): | |||
| if folder.exists(): | |||
| shutil.rmtree(folder) | |||
|
|
|||
| rec_corrected = correct_motion(rec, folder=folder) | |||
| rec_corrected = correct_motion(rec, folder=folder, estimate_motion_kwargs={"win_step_um": 20}) | |||
There was a problem hiding this comment.
the test recording depth is not that high!
|
One last fix needed to use |
|
In last commit: propagate API changes and docs |
@cwindolf
Here the initial port of dredgelib into spikeinterface.
This come with some small refactoring in spikeinterface and folder reorganization.
We should be able to do:
estimate_motion(method="drege_lfp")estimate_motion(method="drege_ap")The come a a quite big refactoring to split methods and into files and many variable names change to align with dredge in spikeinterface and sometimes the align dredge with spikeinterface.
The entire dredge is in one unique file dredge.py