Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix subidx to compute motion correction template from a movie slice #1

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

ludovicbellier
Copy link

When motion correcting a series of recordings (i.e., concatenated movies), in some cases there is a shift between movies' FOVs, and the default template/reference frame computed by averaging 50 frames uniformly sampled across the series is spoiled. Concretely, it's like averaging (for two movies of equal duration) half frames with a "resting position" (x, y) and half frames with a shifted resting position (x + a, y + b), resulting in a blurry / dual state template and eventually to failed motion correction.

One solution is to compute the template from one movie only, and that's what the argument "subidx" is meant to do: define a movie slice from which to sample the 50 frames. Only, 1) this argument wasn't taken into account, and 2) it also affected the output movie (only output the chosen slice), making it useless.

This PR fixes "subidx", and allows for a successful template/reference frame computation (using the exact same sampling and spatial filtering steps as built-in) from one movie only, critically enabling the successful use of NoRMCorre on concatenated movies.

num_splits=num_splits_to_process, shifts_opencv=shifts_opencv, nonneg_movie=nonneg_movie, gSig_filt=gSig_filt,
use_cuda=use_cuda, border_nan=border_nan, var_name_hdf5=var_name_hdf5, is3D=is3D,
indices=indices)
indices=indices, subidx=None)

Choose a reason for hiding this comment

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

FYI subidx=None by default in motion_correction_piecewise, but no harm in stating that explicitly here

Copy link
Author

Choose a reason for hiding this comment

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

Right, and I don't know why they had subidx=subidx in the function call, as it tells motion correction to run just on the specified slice (that's what was making subidx dual function, one the one hand to constrict frames from which to compute the template, and on the other hand by computing motion correction on this only). Writing this I think they maybe never had the use case of wanting to apply a template on a superset of frames, if it makes sense! So maybe they just put that here to say "if I have a movie and just want to do MC from frame A to frame B, I can do so" and it impacted both template computation and MC itself.

@@ -1,7 +1,7 @@
channels:
- conda-forge
dependencies:
- python >=3.10
- python =3.9

Choose a reason for hiding this comment

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

great! we should add specific versions for all dependencies as we make improvements (e.g. I know some older versions of numpy don't work)

Copy link

@zzbalews zzbalews left a comment

Choose a reason for hiding this comment

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

Great!

@ludovicbellier ludovicbellier merged commit eb35738 into master Apr 28, 2023
@ludovicbellier ludovicbellier deleted the subidx branch April 28, 2023 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants