-
Notifications
You must be signed in to change notification settings - Fork 272
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
Fix diffusion mask in voxels outside the FOV #129
Fix diffusion mask in voxels outside the FOV #129
Conversation
If the mask's maximum value is 1 before resampling, thresholding at exactly 1 makes me nervous, as you are expecting the 8 interpolation factors to always sum to exactly 1 and never 0.999999. Thresholding at 0.999 should be functionally equivalent to what is desired, while allowing plenty of room for floating point rounding error. Of course, if fsl's thresholding already has a baked-in rounding error margin, or their trilinear interpolation implementation is very careful, and they will never change them, 1 may be fine as-is. I did include a tiny rounding margin in wb_command -*-math's equality tests, though I'm not sure it would be big enough for the worst cases of multiply and add rounding in some possible implementations of trilinear interpolation. |
Can you maybe include an image of Also, just to clarify, |
And one other thought: It would be great if we could quantify the fraction of voxels that were zeroed-out due to movement, and put that into a text file to promote a simple QC check. |
#Register diffusion data to T1w space. Account for gradient nonlinearities if requested | ||
if [ ${GdcorrectionFlag} -eq 1 ]; then | ||
echo "Correcting Diffusion data for gradient nonlinearities and registering to structural space" | ||
${FSLDIR}/bin/convertwarp --rel --relout --warp1="$DataDirectory"/warped/fullWarp --postmat="$WorkingDirectory"/diff2str.mat --ref="$T1wRestoreImage"_${DiffRes} --out="$WorkingDirectory"/grad_unwarp_diff2str | ||
${FSLDIR}/bin/applywarp --rel -i "$DataDirectory"/warped/data_warped -r "$T1wRestoreImage"_${DiffRes} -w "$WorkingDirectory"/grad_unwarp_diff2str --interp=spline -o "$T1wOutputDirectory"/data | ||
${CARET7DIR}/wb_command -volume_dilate $DataDirectory/warped/data_warped $DILATE_DISTANCE NEAREST $DataDirectory/warped/data_warped_dilated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The command is -volume-dilate
, no underscores. I suggest at least trying the weighted method with -exponent 7
when you test this, for comparison. However, both methods are somewhat slow in 1.3.2, so I would suggest taking a somewhat-dilated brain mask, finding its intersection with the fov-introduced zeros, and using that as the -bad-voxel-roi
argument, to speed things up a bit (by not dilating unnecessary locations).
Also, wb_command requires the complete filename, including extension, so add .nii.gz to them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching the typos. The computation speed is indeed a major problem. I tested both versions of the dilation (resulting b0 attached) and it took 1 hour for NEAREST and 5 hours for WEIGHTED with exponent of 7. The weighted version does look a bit better, but it does not seem worth the extra 4 hours of computation time as the effect on the spline interpolation will probably be very small (I have not tested that yet).
I thought $DILATE_DISTANCE would essentially do what you suggest to do with the -bad-voxel-roi, i.e., by setting it to 12 * 1.25 I only compute the dilation for voxels withing 12 voxels of the FOV. I'll test if the run time is shortened by using -bad-voxel-roi.
I'm still trying to figure out how to set the extent of the dilation. There is no hard boundary in how far sharp boundaries are propogated in the spline. 12 voxels is probably overkill, but I'm not sure at all how far down I can safely go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've had a look at the code for -volume-dilate and it appears to me that for this specific application it is very inefficient, because it iterates over all volumes independently. However, in this case the -bad-voxel-roi and -data-roi are the same for all volumes, so once it has determined for a voxel the nearest neighbour or weights in the kernel for a single volume, it could apply it to all volumes.
I guess if we're really worried about runtime we could (1) assign each voxel in -data-roi an index, (2) run -volume-dilate with NEAREST flag, (3) assign diffusion data to each voxel based on its index. That should be a lot faster than running -volume-dilate on the whole diffusion dataset at once, but would not work with the WEIGHTED flag.
I might just be overthinking it and should just compute the dilation for a more limited -bad-voxel-roi as you suggested and ignore the inefficient runtime...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The smaller the dilation distance, the faster the command will be. Even 1 voxel would be an improvement over the previous results, but 4 might be enough to get most of the possible benefits. I suggested intersecting the negation of the FoV with a somewhat dilated brain mask specifically because it should pinpoint only the voxels contributing to this strong localized ringing effect (and the fewer voxels that -volume-dilate has to fill in, the faster it runs).
The latest code in github is already faster than the 1.3.2 release (it now uses an octtree of only the valid source voxels and uses a kernel cutoff distance based on the closest voxel, 1.3.2 used full-size stencils everywhere and checked against the ROIs repeatedly). You can try the dev_latest here if you want to compare the timing to 1.3.2:
http://brainvis.wustl.edu/workbench/
Cache locality effects might prevent the "each voxel across all volumes" approach from being noticeably faster, and unfortunately the case without a -bad-voxel-roi or -data-roi can have a different set of valid voxels in every frame, so the code would need to be more complicated to support this optimization (it would be better to save the set of source voxels for each output voxel and still go frame by frame).
If you aren't concerned about the values created by the dilation, you might as well just use fsl's dilation, if it is fast. Weighted dilation will always be slower, since it takes data from more than one source voxel for every output voxel. There is also the possibility of -volume-smoothing with the -fix-zeros option (because gaussian smoothing in an orthogonal grid decomposes into 1D smoothings, it is faster - the weighted dilation is trickier because it doesn't use a fixed-size gaussian, but instead a kernel that is invariant to scaling), though that smoothing method assumes that the data can't have real zeros in it.
I'm a bit lost here, why are we dilating? |
Because in the diffusion MRI data from eddy there is a sharp edge at the edge of the field of view (FoV). Voxels which in any volume are outside of the FoV are set to zero. The spline interpolation used in the resampling to T1w space is very sensitive to these type of sharp edges, which leads to striping artifacts. Dilation will move such striping artifacts outwards, which minimizes their effect on those voxels fully within the FoV (which we want to keep). |
So a few iterations of nearest neighbor should be fine if you will be remasking anyway? |
Yeah, that would probably be enough. Nearest neighbour across 4 voxels (using -volume-dilate) took a bit less that 10 minutes, so that seems reasonable. |
${FSLDIR}/bin/flirt -in "$DataDirectory"/data_dilated -ref "$T1wRestoreImage"_${DiffRes} -applyxfm -init "$WorkingDirectory"/diff2str.mat -interp spline -out "$T1wOutputDirectory"/data | ||
|
||
#Create a mask covering voxels within the field of view for all volumes | ||
${FSLDIR}/bin/fslmaths "$DataDirectory"/nodif -abs -bin "$DataDirectory"/fov_mask |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I suggested using nodif
as a proxy for the internal fov mask within eddy (which, ideally, would write out the fov mask as an explicit output). But do we need to be concerned about the possibility of randomly getting a "true zero" within the first frame that wasn't actually part of the eddy
internal fov mask?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've addressed this issue by actually checking for the -Tmax after taking the absolute value in the data. So we will only exclude voxels containing only zeroes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe my concern about this possibility was misplaced, since getting a true zero is very unlikely. The problem with the -Tmax
approach is what if there are zeros in say half the frames for a given voxel, then we don't want to use that voxel. I guess that scenario shouldn't happen if eddy
itself is enforcing a common fov mask across all frames. But I think it is better if we write this code to be robust to whatever eddy
chooses to do in that regard. Which would argue for not using nodif
as a proxy for the internal eddy
fov mask, and using -Tmin
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-Tmin
means if even a single frame has an exact zero there, it is considered bad, so it seems worse than the original simpler tactic of looking at just the first frame.
If we want a more robust solution, what if we thresholded to find all exact zeros, counted the number of frames a voxel is zero with a sum operation, and then used a second threshold (say, at least 10% of frames) on that?
However, if we can in fact model the FoV as a mask directly without depending on exact zeros in the output, that would resolve all such concerns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, unless we think eddy is likely to get changed, I don't think we should be coding for what it might be changed to do, so if it does change to per-volume masking, we should expect to have to rewrite this code anyway (and the correct thing to do then would probably be separate ROIs per frame).
@MichielCottaar See #138 for some files that I'm creating to allow us to quickly assess the spatial coverage issue with the fMRI data. Ideally, the changes you are working on would incorporate something similar for quickly assessing the coverage in the dMRI data. I'd like to get that PR wrapped up early next week, and it would be nice if we could get this PR done at the same time as well. |
As suggested by Tim Coalson. Not sure if it is needed, but might as well.
Gives equivalent results, but is faster
ab9cad8
to
26108f6
Compare
I've rebased onto master, hence all the repeat of the commits. @mharms Sure, the latest commit should add such a text file. I'm just running the new pipeline with and without gradient non-linearities (see https://git.fmrib.ox.ac.uk/ndcn0236/mask_striping). If that works, I'm happy for this to be merged. |
Ok, this version runs on jalapeno. It produces a summary of the coverage, which for my test subject contains:
|
However, while looking through the code, I thought of three other things:
|
Expanding on my point 3 above, I see that we also do the same |
I have moved the FoV mask calculation to post_eddy and added a dilation step when interpolating the diffusion data in post_eddy (points 1 and 2 from Mike). I'll need to rerun the code on jalapeno to see if any new bugs have arisen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just what I was thinking, but there is a bug in one line that is going to prevent the $GdCoeffs='NONE'
condition from working in eddy_postproc.sh
. Once that is fixed, and both the GDC and no GDC conditions are confirmed to work, this is ready to go in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once we fix the lingering bug, and the GDC and no GDC conditions are both confirmed to work, this looks ready to me.
With these latest fixes both the GDC and no GDC conditions have produced T1-aligned diffusion data with the FoV mask applied. |
The voxels inside the FOV at the first b0 might not be included in some other volumes due to movement. In eddy the signal these voxels, which are not included in all the volumes, are set to zero. In the HCP pipeline the output of eddy is resampled to T1 space. During this resampling, these voxels become non-zero due to the spline interpolation and are included within the final nodif_brain_mask.
This patch fixes this by:
This image shows the change in the final mask for subject 101309, which was the first subject where I noticed this issue, when looking at DTI fits. Note the very low range of the colour bar when plotting the B0 on the left. This is to visualise the striping caused by the spline interpolation.