Skip to content

Refactor correct_motion into compute motion, then make interpolation#3905

Merged
alejoe91 merged 19 commits intoSpikeInterface:mainfrom
chrishalcrow:add-estimate-motion
May 7, 2025
Merged

Refactor correct_motion into compute motion, then make interpolation#3905
alejoe91 merged 19 commits intoSpikeInterface:mainfrom
chrishalcrow:add-estimate-motion

Conversation

@chrishalcrow
Copy link
Copy Markdown
Member

PR to re-factor the correct_motion function into two parts: compute_motion (which returns just a motion_info object) and interpolate_motion.

We expose the compute_motion function and edit the docs to use it. This encourages users to do the motion correction computation and check their results before interpolation (as Sam always tells us to do!). At the moment, to do this, the user has to do

_, motion, motion_info = si.correct_motion(recording=rec, preset="dredge_fast", output_motion=True, output_motion_info=True)

Then analyse the results. Now they can do

motion_info = si.compute_motion(recording=rec, preset="dredge_fast")

Which hopefully feels nicer.

Alessio also requested a raise_error argument, so that if the estimate_motion fails, the peaks and peak locations are still returned. Motion estimation often fails on e.g. a single shank-bank of a NP probe. But it can still be helpful to do a raster plot of the peaks. Hence this seems helpful. Note: it returns motion_info['motion'] = None at the moment.

I've done some little refactors, and updated some things to improve provenance. These are also in preparation for adding a new class ComputeMotionAndInterpolateRecording soon.

The correct_motion function should work exactly as before.

@chrishalcrow chrishalcrow added the motion correction Questions related to motion correction label May 6, 2025
@chrishalcrow chrishalcrow changed the title Add estimate motion Refactor correct_motion into compute motion, then make interpolation May 6, 2025
detect_peaks_method = detect_kwargs["method"]
method_class = detect_peak_methods[detect_peaks_method]
detect_kwargs_without_method = {
key: detect_kwarg for key, detect_kwarg in detect_kwargs.items() if key != "method"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous code was poping some kwargs from the dict, which pop'd them out of the parameters dict, losing provenance. We now make a detect_kwargs_without_method so that we don't edit the detect_kwargs dict,

raise RuntimeError(error_message)
else:
warnings.warn(error_message)
motion = None
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If raise_error is set to False and the estimate_motion function fails, the function still returns motion_info, just with motion = None.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was my wish! Thank you! :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the save_motion_info / and load_motion_info should also be updated to handle motion=None. Maybe worth adding a test with a specified folder?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - now if motion = None, save_motion_info makes a folder with no motion subfolder. When load_motion_info finds this folder structure, it sets motion to None.

@chrishalcrow chrishalcrow requested a review from alejoe91 May 6, 2025 16:34
Copy link
Copy Markdown
Member

@alejoe91 alejoe91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some tiny comments! Looks super :)

Have you tried medicine in the handle drift tutorial?

Comment thread src/spikeinterface/preprocessing/tests/test_motion.py Outdated
Comment thread examples/how_to/handle_drift.py
Comment thread src/spikeinterface/preprocessing/motion.py Outdated
raise RuntimeError(error_message)
else:
warnings.warn(error_message)
motion = None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was my wish! Thank you! :)

Comment thread src/spikeinterface/preprocessing/motion.py Outdated
Co-authored-by: Alessio Buccino <alejoe9187@gmail.com>
@alejoe91 alejoe91 added this to the 0.102.3 milestone May 6, 2025
@chrishalcrow
Copy link
Copy Markdown
Member Author

Hello, I'm trying medicine now - might be easier to leave it for another PR (I'll working more on this very soon!)

Comment thread src/spikeinterface/preprocessing/motion.py
Comment thread src/spikeinterface/preprocessing/motion.py
@alejoe91 alejoe91 merged commit c1dad62 into SpikeInterface:main May 7, 2025
15 checks passed
@chrishalcrow chrishalcrow deleted the add-estimate-motion branch July 30, 2025 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

motion correction Questions related to motion correction

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants