Skip to content
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

fMRIVolume Related Pull Request — Use of a specific BOLD mask #141

Closed

Conversation

sivcek
Copy link
Contributor

@sivcek sivcek commented Oct 3, 2019

Adds the (optional) ability to specify the mask to use in the final Intensity Normalisation step. The options are:

T1 - the default option
BOLD - a mask created by bet on the mean bold image across the timeseries
DILATED - use the MNI152_T1_2mm_brain_mask_dil.nii.gz mask
NONE - do not use a mask

In our experience using a wider mask than T1 (DILATED or NONE) helped in checking the quality of bold registration, as it was possible to see if the BOLD image was expanded beyond the borders defined by the T1w mask.

@sivcek sivcek changed the title Qunex fmrivolume mask fMRIVolume Related Pull Request — Use of a specific BOLD mask Oct 4, 2019
Compliance="HCPStyleData"
ComplianceMsg=""

if [ ! "${Mask}" = 'T1' ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the purpose of the mask is more to reduce file sizes by setting non-brain to exact 0s, so I'm not sure this needs to be restricted to legacy data mode.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, for intensity normalization...depending on how we do that, it may be important to use a tight mask.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if there is a way to compute the intensity normalization using a tighter mask and then apply it on a wider one.

Copy link
Contributor

@glasserm glasserm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I don't really like this PR. If you need to, just dilate the existing brain mask (which will be maximally accurate to the given brain). I don't see any reason to invoke the MNI space mask or recompute it from the BOLD data (potentially masking out dropout regions). It probably would be possible to replicate the intensity normalization using wb_commands and gain control of the masking. I agree that intensity normalization would be influenced by dilation of the mask. Also, I am not sure I understand how in a rigid registration the BOLD image would expand, so I am unclear on the need for the PR in any case. If there is a strong feeling to keep it, then the options would be T1w, dilated T1w, or NONE.

@glasserm glasserm requested a review from mharms October 6, 2019 16:01
@sivcek
Copy link
Contributor Author

sivcek commented Oct 8, 2019

@glasserm "expand" is not the right term. With lack of DC, expanded mask allows to see how badly aligned the image is.

@glasserm
Copy link
Contributor

glasserm commented Oct 9, 2019

Let's implement as I suggested above

@mharms
Copy link
Contributor

mharms commented Oct 9, 2019

The purpose as stated in the PR was to "help in checking the quality of bold registration". If that's the goal, I think there are better, more explicit ways we can check the registration than to expand the mask (which seems like a crude solution) -- e.g., we can make sure that we make available a non-masked version of the registered Scout, so that you can easily visualize what is occurring "beyond the borders defined by the T1w mask". Also, @sivcek, this is going to interact with PR #138 which I'm hoping to merge into the master tomorrow.

@glasserm
Copy link
Contributor

glasserm commented Oct 9, 2019

Well I also agree with this.

@sivcek
Copy link
Contributor Author

sivcek commented Oct 9, 2019

@glasserm @mharms The value goes beyond just QC checking. If somebody want's to stay for some reason in volume-land, they might want to make sure that the BOLD was not cut and would still proceed with suboptimal registration, that would be prohibitive for surface analysis, but might be usable for volume-based analyses. If it's ok with you. I'll wait for @mharms to pull in his PR and I'll see how to re-introduce the functionality to satisfy previous comments.

@mharms
Copy link
Contributor

mharms commented Oct 9, 2019

The PR was merged last night, so check out the current master branch, and proceed from there.

@sivcek
Copy link
Contributor Author

sivcek commented Oct 9, 2019

Ok.

@glasserm glasserm requested a review from coalsont October 10, 2019 01:26
@glasserm
Copy link
Contributor

If we are keeping it, we have the default tight brain mask, a dilated version of this mask, and no mask as reasonable options.

@sivcek
Copy link
Contributor Author

sivcek commented Oct 12, 2019

I have now added the following options:

  • FINAL (the default)
  • T1 (for T1w image based mask)
  • DILATEDT1 (for dilated T1w based mask)
  • NONE (for no mask)

I've also implemented checks for valid options.

@sivcek
Copy link
Contributor Author

sivcek commented Oct 12, 2019

FINAL is the mask created by code that @mharms added.

@glasserm
Copy link
Contributor

So final is a combination of the T1w mask and the FOV mask, right?

@glasserm
Copy link
Contributor

Okay, lets call it T1_fMRI_FOV

Copy link
Contributor

@mharms mharms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, IMO, the "FOV mask" (${InputfMRI}_mask) should always be applied, regardless of whether you want a more generous T1-related mask -- under what situation is it valid to include the voxels that are outside of the "FOV mask" (i.e., that don't have proper spatial coverage across all time points)?

If the FOV mask is always applied, then the T1 option becomes irrelevant.


if [ "${UseMask}" = "DILATEDT1" ] ; then
# -- create a dilated version of T1 mask
${FSLDIR}/bin/fslmaths ${BrainMask} -dilF ${BrainMask}_dil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a single round of dilation enough to accomplish your purposes with the DILATEDT1 mask? Seems like you'd want more than that...

Copy link
Contributor Author

@sivcek sivcek Oct 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The options now will be:

  • T1_fMRI_FOV: combined T1w brain mask and FOV masks (the default),
  • T1_DILATED_fMRI_FOV: a once dilated T1w brain based mask combined with fMRI FOV
  • T1_DILATED2x_fMRI_FOV: a twice dilated T1w brain based mask combined with fMRI FOV,
  • fMRI_FOV: a fMRI FOV mask

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have also changed the flow of the code so that the FinalMask is the mask specified. And that the statistics are reported both on the FOV of the T1w brain as well as the actual mask used, with the mask used also listed in the ${FinalMask}.stats.txt

…DILATED_fMRI_FOV, T1_DILATED2_fMRI_FOV, fMRI_FOV, and expanded statistics report
@sivcek
Copy link
Contributor Author

sivcek commented Oct 14, 2019

@mharms @glasserm I hope that your requests are now satisfied.

@glasserm glasserm requested a review from mharms October 15, 2019 01:18
@sivcek
Copy link
Contributor Author

sivcek commented Oct 15, 2019

@mharms, @coalsont I'll combine this PR with the rest fMRIVolume ones once also the reference BOLD PR is done.

@sivcek
Copy link
Contributor Author

sivcek commented Nov 7, 2019

Joined with other fMRIVolume related PRs into PR #156

@sivcek sivcek closed this Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants