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

Investigate if we are handling motion borders consistently #458

Open
danielsf opened this issue Feb 18, 2022 · 0 comments
Open

Investigate if we are handling motion borders consistently #458

danielsf opened this issue Feb 18, 2022 · 0 comments
Labels
bug Something isn't working

Comments

@danielsf
Copy link
Contributor

Looking at the MotionBorder class defined in ophys_etl/utils/motion_border.py, I discovered that it does not contain data about the motion border. It contains data about the maximum shift applied to the movie in any given direction. See, for instance

https://github.com/AllenInstitute/ophys_etl_pipelines/blob/main/src/ophys_etl/utils/motion_border.py#L10-L60

Assuming it passes code review, I am going to rename that class MaxFrameShift and add a MotionBorder class that actually does contain information about the motion border (the positive number of pixels to ignore on any given edge of the field of view). This should be a part of #456 (assuming it passes code review).

In the course of making that change, I became concerned that we may not be consistently defining motion borders in our codebase. See, for instance, this code

https://github.com/AllenInstitute/ophys_etl_pipelines/blob/main/src/ophys_etl/utils/roi_masks.py#L240-L266

The docstring defines the zeroth element of border as "the border width on the right" but the code uses border[0] to determine if an ROI is too far to the left. This arises from the "motion border versus maximum frame shift" ambiguity. If a frame is shifted to the left during motion correction, that will make pixels on the right untrustworthy (either because of padding or pixel wrapping).

I think we need to go through our codebase and disambiguate where we mean a motion border (a positive definite number of pixels to ignore at a given edge of the field of view) and where we mean the maximum shift applied to a frame in a given direction during motion correction.

@danielsf danielsf added the bug Something isn't working label Feb 18, 2022
danielsf added a commit that referenced this issue Feb 18, 2022
The second arg of np.max is actually an axis, so these
calls to np.max were not clipping the max shifts to be >= 0
(in fact, if you look at tests/utils/test_motion_border.py,
our unit tests were written to expect negative values as a
legal output).

Probably this was not intended, but, all of our code has
been built around the current implementation, so I'm not going
to alter the logic in get_max_correction_values now.

Ticket #458 has been opened to do a more thorough and
self-consistent investigation of our treatment of motion borders
in the future. At that time, we can consider altering
get_max_correction_values to only return values >= 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant