Washington-University / NHPPipelines

Enhancements of HCP Pipelines for non-human primates
Other
24 stars 20 forks source link

New CustomMask options? #6

Open takuya-hayashi opened 2 years ago

takuya-hayashi commented 2 years ago

https://github.com/Washington-University/NHPPipelines/blob/29f75cdf2d9447e9381c6fa48cfa76cf504bf6ab/PreFreeSurfer/PreFreeSurferPipeline.sh#L773-L787

@coalsont @glasserm

Since I use the NHP brain mask in the original space by default, I would modify the pipeline to add an option to use the mask in the original space in addition to the current CustomMask option to use the mask in the ACPC space. So I would like to propose creating a new option of 'Mask_orig' for our purpose and using 'Mask_acpc' instead of 'Mask' in previous version of option. What do you think?

coalsont commented 2 years ago

MASK is currently a valid setting for a user-specified option, I think it should continue to be accepted, and do what it did previously. We can have MASK and MASK_acpc simply do the same thing, if we prefer to have MASK_acpc be an additional valid setting:

...
elif [[ "$CustomBrain" == "MASK_acpc" || "$CustomBrain" == "MASK" ]] ; then
...

Side note: this syntax is one of the sharp corners of bash, you have to use [[ to put the || there, [ x || y ] doesn't work. [ x ] || [ y ] is how you would need to do it with [. This is one reason I always write [[.

takuya-hayashi commented 2 years ago

Okay. Then, I would change the following line (L495):

From

if [ "$CustomBrain" = "NONE" ] ; then

To

if [[ "$CustomBrain" = "NONE" || "$CustomBrain" = "Mask_orig" ]] ; then

because the 'Mask_orig' option shares many functions as already used in the option of 'NONE'

coalsont commented 2 years ago

The current code looks like it uses TXwFolder without setting it in the MASK_orig case, so it shouldn't run properly, I assume you are still working on it. Normally, I wouldn't recommend mixing cases that have some differences, but if it needs to do everything that NONE does with only minor tweaks, then maybe the code it would save would be worth it.

How do you handle the difference in position between T1w and T2w if your mask is in scanner space? What do you gain by generating the mask in scanner space?

takuya-hayashi commented 2 years ago

Yes, I'm still thinking of it but not yet coded. I think almost everything that the current NONE does is also useful even using Mask_orig, but with the minor tweak.

I would use the separate masks for T1w and T2w (e.g. t1_brain.nii.gz , t2_brain.nii.gz) as an initialization of the ACPCAlignment, which quite often fails if we don't use brain only image for the initialization registration (w/ the reference target of brain only template). Probably this is because the volumes of non-brain structures are relatively large and often variable across individuals (e.g. muscle and eyes) and signals are often very high (eye).

glasserm commented 2 years ago

This sounds like an option that was added by the Qu|Nex folks as I didn't remember it. I agree with Tim that we should try as much as we can to not break backwards compatibility. Also I recommend that we test code before merging it into the HCP Pipelines. Thank you both for your work on this.

takuya-hayashi commented 2 years ago

We will try keeping backwards compatibility. Thanks!