-
Notifications
You must be signed in to change notification settings - Fork 181
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
Population template: mrgrid crop -mask dilated and explicit mrtransform reorient #1678
Conversation
mrmath added option to not wipe axes with single elements needed for population_template with 4D input with 1 volume
While test currently fails, this should hopefully be addressed following merge of #1678.
* As opposed to mrcrop, mrgrid applies margin padding beyond FOV (and supports signed margin) * This reverts population_template: dilate mask before cropping to mask bdaa52f.
cd51eb7
to
ecd2f66
Compare
This reverts to the old One difference is that cropping to a mask with a positive margin can now also extend the FOV of the cropped image if the mask + margin extends beyond the edge of the FOV. When interpolating at the very edge of the image, this can bias image intensities towards zero but allows using non-nearest interpolation. Any opinions on potentially padding beyond the FOV when cropping to a mask image? |
Can this be merged? |
…efault restricted to valid FOV
On second thought and discussion with Daan, I think it is best to (550e93a):
To illustrate the difference that padding the FOV before rigid (rotation) transformation and regridding makes: Cropping to mask and rigid transformation with regridding (linear interpolation) gives and for "unbound padding" (before 550e93a, cropping with 1 voxel padding irrespective of FOV): Regridded images are identical if nearest interpolation is used. Difference between the two linearly interpolated images (mask is nearest-interpolated): transformed mask without regridding: Transforming the regridded images back without regridding to assess the mean squared difference to the original in scanner space inside the original mask or across the whole image using linear or nearest interpolation (
So, when evaluated in the original scanner space, padding at the edge of the FOV before transformation gives a lower error for evaluation using linear interpolation but higher error when evaluated with nearest interpolation. Or in other words, the bound padding behaviour is better if we care more about voxel space (voxel-wise average in population_template), the crop_unbound is better if we care about the scanner space after transformation. Feel free to merge or object to any of the changes! |
Pretty sure I understand. But the functionality is kinda funky and won't be easy to follow for many. I'm increasingly moving longer explanations to the DESCRIPTION section rather than trying to shove stuff into the option helps; I'm wondering if that might be useful here. I'd also considering renaming |
Given that the theme of this pull request is basically to make |
|
What part of the functionality do you mean? If you refer to the added restriction that crop operations do not pad unless explicitly specified, yes the lack of
This command crops to mask then extends the resulting FOV by 5 voxels (-1 around mask, -4 added) in all directions (up to the original FOV) then resets the extent of the z axis. So, the lack of Maybe better to change the
Feel free to modify as you see fit! I tried to:
No strong preference but I think I prefer
Not sure if I break the changes to testing/script_tests/population_template when I cherry-pick 2a97a30 here? |
"funky" wasn't exactly my most articulate moment :-/ It can take a moment to wrap one's head around the concepts and therefore the consequences of different usages. Also, while
No, I don't think failing to specify an option should produce a warning if the only crime was providing a mask that runs all the way to the edge of the FoV. But maybe given the complexity we could add a
I could resolve such if you want to. Otherwise just merge this first, and I'll shortly after merge #1614. |
Ok, I modified:
Example:
Docs:
|
👍 Great stuff; looks like this fixes everything. |
- mrthreshold: Fix erroneous exception message introduced in #1636. - dwi2response msmt_5tt: Compatibility fix for low-resolution data (ie. few voxels), when running dwi2response tournier, based on new mrthreshold -top implementation in #1636. - 5ttgen: Fixes to FoV cropping due to behaviour of mrgrid crop, which deviated from former command mrcrop in #1609 but was restored to previous behaviour in #1678.
dwinormalise group data required updating, just as did population_template, for which new data were brought in in 33f2291.
changes to address issues with shrinking templates #1676
mrcrop
-mask
reverted behaviour to pad by 1 voxel on all sides by default (new option-mask_margin
).population_template
if the number of volumes matches SH series after mrtransform Jacobian modulation and explicit reorientation #1598mrmath
: added-keep_unary_axes
option and use that inpopulation_template
to avoid dimension mismatch between template and input data