Resample lesion mask into in_file's voxel grid#1050
Draft
tsalo wants to merge 2 commits into
Draft
Conversation
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
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.
Closes #1021.
Summary
create_cfm()builds the cost-function mask used to constrain registration by subtracting a lesion mask from a global/brain mask. It loaded the lesion vianb.as_closest_canonical(), reorienting it to RAS, while leavingin_filein its stored orientation. Whenin_fileis not RAS, the two voxel arrays are in different orders, so the lesion gets subtracted from the wrong voxels — the lesion has no effect at its true location and spurious differences appear elsewhere.This reorients/resamples the lesion mask into
in_file's exact voxel grid withnibabel.processing.resample_from_to(..., order=0)before subtracting. Nearest-neighbor keeps the mask binary, and resampling by world coordinates makes the result correct for any orientation or grid — not just RAS.Reported downstream in PennLINC/qsiprep#1023, where the normalization runs in LPS and every lesion ROI hit this bug.
Changes
niworkflows/interfaces/norm.py: replaceas_closest_canonicallesion handling increate_cfmwithresample_from_to(..., order=0); update the now-inaccurate docstring note that claimed the inputs must share a grid.niworkflows/interfaces/tests/test_norm.py: addtest_create_cfm_lesion_orientation, which builds an LPSin_fileand an RAS single-voxel lesion at a known world coordinate and asserts the cost-function mask is excluded at exactly that world location. Fails on the old code (excluded voxel lands at the mirrored location), passes with the fix.Testing
pytest niworkflows/interfaces/tests/test_norm.py— passes (incl. the new regression test); no other tests affected.🤖 Generated with Claude Code