Repository navigation
fix: use the right axis for motion correction shift bookkeeping - #141
Merged
Merged
Conversation
The displacement field of FlowRegistration holds the displacement in x in its first channel and the displacement in y in its second, which is the order imwarp takes a displacement field in. updateCorrectionStats read the channels the other way round, so offsetX and offsetY in the saved MotionCorrectionStats were swapped. The image data was never affected, because the correction applies the field itself and not these values. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
correctDrift returns the drift as [dx, dy], while normcorre follows the MATLAB array convention and holds the shift in y in the first plane of its shifts and the shift in x in the second. addShifts added dx to the y plane and dy to the x plane, so the drift ended up on the wrong axis of the shifts that are saved as NormcorreShifts. The image data was never affected, because correctDrift translates the frames before the drift is added to the shifts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Test Results369 tests 369 ✅ 25s ⏱️ Results for commit 5b07c60. |
ehennestad
commented
Sep 18, 2026
Collaborator
Author
There was a problem hiding this comment.
Both confirmed.
W (displacement matrix) is not documented in flowreg, but it is passed to imwarp in its original form. imwarp's documentation is clear, 1st dimension is x, 2nd dimension is y.
https://se.mathworks.com/help/images/ref/imwarp.html#btqog63-1-D
correctDrift does output [dx, dy] and reversing these before adding to normcorre shifts on save is correct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two places in the motion correction wrappers mix up which component of a shift is x and which is y. Both affect the values that are saved alongside a correction, not the corrected image data.
nansen.wrapper.flowreg.Processor.updateCorrectionStatsread the displacement field as if its first channel were the displacement in y. FlowRegistration returns the field in the order imwarp takes one, with x in the first channel and y in the second, sooffsetXandoffsetYin the savedMotionCorrectionStatswere swapped. The code carried aTodo: Is 2nd x-offsets?comment on exactly these two lines. I checked the order by registering frames with a known translation: for a frame moved 3 pixels in x, the mean of the first channel is 3.01 and the mean of the second is 0.00.nansen.wrapper.normcorre.Processor.addShiftsadds the drift thatcorrectDriftmeasured to the shifts of a part.correctDriftreturns[dx, dy]while normcorre follows the MATLAB array convention and holds the shift in y first, so the drift was added to the opposite axis of the savedNormcorreShifts. This only happens when drift correction runs, which is from the second part onwards. The frames themselves are translated bycorrectDriftbefore the shifts are touched, so the correction that was applied was right and only the record of it was wrong.The flowreg version of
addShiftswas already consistent withcorrectDrift; it now says so in a comment, because the two wrappers take the components in opposite order and that is easy to "fix" in the wrong direction.The two fixes are separate commits, so either can be dropped.
🤖 Generated with Claude Code