You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
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
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.
The text was updated successfully, but these errors were encountered:
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
Looking at the
MotionBorder
class defined inophys_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 instancehttps://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 aMotionBorder
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 usesborder[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.
The text was updated successfully, but these errors were encountered: