FreeSurfer Pipeline related pull request

[sivcek]

FreeSurferPipeline.sh related changes

1/ Addition of optional —mppversion parameter

The parameter has the same functionality as in the PreFreeSurfer pipeline. And is related to the MPP version checking, which in this case only checks for the presence of T2w image.

2/ Ability to process w/o T2w image

If T2w image is set to NONE and legacy MPP version is selected then:

• -T2 option is excluded from recon_all_cmd
• -T2pial is replaced with -noT2pial in recon_all_cmd
• Making T2w to T1w registration available in FSL format section is skipped
• make_t2w_hires_nifti_file and make_t1wxtw2_qc_file is skipped

3/ Cosmetic changes

Switch from tabs to spaces and remove trailing line spaces

FreeSurferHiresPial.sh related changes

1/ Cosmetic changes

Switching from tabs to spaces, removing trailing spaces in lines

2/ Skipping operations that depend on T2w image

When T2wImage is set to NONE, operations that depend on the presence of T2wImage are skipped.:

• creation of normalized T2w image
• generation of pial first surface with T2 adjustment
• generation of pial second pass surface with T2 adjustment

3/ Resolving a bug with computing c_ras.mat

We identified a bug in a code that was triggered in some cases and replaced it with alternative more robust implementation (lines 104-112)

4/ The use of ceho is suppressed until debugging library is pulled in

FreeSurferHiresWhite.sh related changes

1/ Cosmetic changes

Switching from tabs to spaces, removing trailing spaces in lines

2/ Skipping operations that depend on T2w image

When T2wImage is set to NONE the following is skipped:

• computing T2 to T1 transform

3/ The use of ceho is suppressed until debugging library is pulled in

[glasserm]

These look fine.

[mharms]

Per the README in the FreeSurfer/scripts directory:

FreeSurferHiresPial.sh and FreeSurferHiresWhite.sh are "legacy" scripts
that support the use of FreeSurfer 5.3.  They are called by the
${HCPPIPEDIR}/FreeSurfer/FreeSurferPipeline-v5.3.0-HCP.sh script.

So, edits to those scripts are not necessary unless you are intending to support the use of no T2 image in the context of FS 5.3 as well. And if that is the case, then edits to FreeSurferPipeline-v5.3.0-HCP.sh will be necessary as well. I suggest we only worry about supporting the use of no T2 in the context of FS 6.0.

[glasserm]

Sure, but they already implemented it, so why take it out?

[mharms]

Because I'd rather we not inadvertently break something in the FS 5.3 related code.

[sivcek]

@mharms we've used v5.3.0-HCP extensively in the past and it has been throughly tested with the legacy data and not T2w image. It would be good to support the "LegacyStyleData" in it, so I have added the MPP check there as well.

[mharms]

The allowance of T2wImage=NONE interacts with the code we have in place regarding FS edits and the if [ "${existing_subject}" != "TRUE" ]; then section therefore needs attention. This will also interact with both PreFreeSurfer and PostFreeSurfer, because the file that we are using as a proxy for whether PostFreeSurfer has been run previously is the warp field encoded in OrigT2wToT1w.nii.gz

[mharms]

For documentation purposes, what was the nature of the bug "in some cases" in the computation of c_ras.mat, and under what situations does it manifest?

[sivcek]

@mharms The problem was when the path to brain.finalsurfs.mgz included any of c_r, c_a, or c_s. In those cases it would get through the grep statement and try to cut the first line of the output. This happened to us in a huge data processing push.

[sivcek]

The allowance of T2wImage=NONE interacts with the code we have in place regarding FS edits and the if [ "${existing_subject}" != "TRUE" ]; then section therefore needs attention. This will also interact with both PreFreeSurfer and PostFreeSurfer, because the file that we are using as a proxy for whether PostFreeSurfer has been run previously is the warp field encoded in OrigT2wToT1w.nii.gz

Any suggestions how to deal with that?

[mharms]

Per my suggestion on the PreFS PR, I suggest we get PostFS into better shape first, dealing with the issues that Matt raised (i.e., we can't just cut out CreateMyelinMaps entirely, because some other important stuff happens in that script besides just the myelin maps). Unfortunately, CreateMyelinMaps is a very dense script, and not well-commented at this particular point in time. Perhaps if either @glasserm or @coalsont could go through that script and split it into clear commented "sub-sections", we could separate out the non-myelin-mapping code into its own separate sub-script called by PostFreeSurferPipeline.sh

[sivcek]

The allowance of T2wImage=NONE interacts with the code we have in place regarding FS edits and the if [ "${existing_subject}" != "TRUE" ]; then section therefore needs attention. This will also interact with both PreFreeSurfer and PostFreeSurfer, because the file that we are using as a proxy for whether PostFreeSurfer has been run previously is the warp field encoded in OrigT2wToT1w.nii.gz

Would it be ok to instead use another proxy for checking whether PostFreeSurfer has been run. E.g. the presence of MNINonLinear/*.spec file?

[mharms]

Regarding the test for whether PostFS has been previously run, I think we can simply change the following 2 lines:
local OutputOrigT2wToT1w="OrigT2w2T1w" #Needs to match name used in PostFreeSurfer (N.B. "OrigT2" here refers to the T2w/T2w.nii.gz file; NOT FreeSurfer's "orig" space)
to
local OutputOrigT1wToT1w="OrigT1w2T1w" #Needs to match name used in PostFreeSurfer (N.B. "OrigT1" here refers to the T1w/T1w.nii.gz file; NOT FreeSurfer's "orig" space)

and
if [ imtest ${SubjectDIR}/xfms/${OutputOrigT2wToT1w} = 1 ]; then
to
if [imtest ${SubjectDIR}/xfms/${OutputOrigT1wToT1w} = 1 ]; then

[sivcek]

I implemented the suggested change in the test for existing PostFreeSurfer run.

[mharms]

This PR seems nearly ready.

[sivcek]

@mharms I believe the only open issue is the question regarding MPP in the help to @coalsont