From 7d717b8f6d5db427d146617afcceeef561700efb Mon Sep 17 00:00:00 2001 From: Michael Harms Date: Thu, 7 Nov 2019 17:35:30 -0600 Subject: [PATCH 1/4] Adapt single-run FIX to allow MotionRegression=FALSE if the FIX version is recent enough --- ICAFIX/hcp_fix | 82 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 77 insertions(+), 5 deletions(-) diff --git a/ICAFIX/hcp_fix b/ICAFIX/hcp_fix index 8b33c3f6f..d372deb53 100755 --- a/ICAFIX/hcp_fix +++ b/ICAFIX/hcp_fix @@ -69,6 +69,55 @@ fi ############################################################# +# Need at least fix version 1.06.12 for proper handling of the do_motion_regression=FALSE condition +# NOTE: +# Don't echo anything in this function other than the last echo +# that outputs the return value +determine_old_or_new_fix() +{ + # Default to "OLD", and change to "NEW" only if version is 1.06.12 or later + local old_or_new="OLD" + + fix_version_file="${FSL_FIXDIR}/fixversion" + + if [ -f "${fix_version_file}" ]; then + + fix_version=$(cat "${fix_version_file}") + + # parse the FIX version information into primary, secondary, and tertiary parts + fix_version_array=(${fix_version//./ }) + + fix_primary_version="${fix_version_array[0]}" + fix_primary_version=${fix_primary_version//[!0-9]/} + + fix_secondary_version="${fix_version_array[1]}" + fix_secondary_version=${fix_secondary_version//[!0-9]/} + + fix_tertiary_version="${fix_version_array[2]}" + fix_tertiary_version=${fix_tertiary_version//[!0-9]/} + + # Important: Use single bracket for following tests, since double bracket results in interpretation + # of a leading zero as an octal number. + # [Alternatively, could force a base-10 interpretation using a "$(( 10#$ver ))" construction]. + if [ ${fix_primary_version} -ge 2 ] ; then + old_or_new="NEW" + elif [ ${fix_primary_version} -eq 1 ] ; then + if [ ${fix_secondary_version} -ge 7 ] ; then + old_or_new="NEW" + elif [ ${fix_secondary_version} -eq 6 ] ; then + if [ ${fix_tertiary_version} -ge 12 ] ; then + old_or_new="NEW" + fi + fi + fi + + fi + + echo ${old_or_new} +} + +############################################################# + # Set global variables g_script_name=$(basename "${0}") @@ -106,6 +155,12 @@ log_Msg "Showing FSL version" fsl_version_get fsl_ver log_Msg "FSL version: ${fsl_ver}" +# Show specific FIX version, if available +if [ -f ${FSL_FIXDIR}/fixversion ]; then + fixversion=$(cat ${FSL_FIXDIR}/fixversion ) + log_Msg "FIX version: $fixversion" +fi + # Log FSL_FIX_MATLAB_MODE (from the settings.sh file) log_Msg "FSL_FIX_MATLAB_MODE: ${FSL_FIX_MATLAB_MODE}" @@ -137,12 +192,17 @@ function interpret_as_bool() } doMotionRegression=$(interpret_as_bool "$3") -# 5/20/2019: For the time being, do_motion_regression=FALSE doesn't work properly with single-run FIX, -# so don't allow that mode of operation +# do_motion_regression=FALSE combined with hp filtering doesn't work properly with single-run FIX, +# for fix versions earlier than 1.06.12, so don't allow that mode of operation if the fix version +# isn't recent enough. # https://github.com/Washington-University/HCPpipelines/issues/108 +# Check and abort early if we can't implement the requested options with the fix version in use. if [[ "$doMotionRegression" == "FALSE" ]] then - log_Err_Abort "Due to a restriction in the FIX argument parsing, do_motion_regression=$3 is not currently supported" + old_or_new_fix=$(determine_old_or_new_fix) + if [ "${old_or_new_fix}" == "OLD" ] && (( hp >= 0 )) ; then + log_Err_Abort "Due to a restriction in the FIX argument parsing, do_motion_regression=$3 is not supported for FIX versions prior to 1.06.12" + fi fi unset TrainingData @@ -341,8 +401,20 @@ if [[ ${doMotionRegression} == "TRUE" ]]; then #use array for whitespace safety, even if the rest of the script isn't fix_cmd=("${FSL_FIXDIR}/fix" "${fmrihp}.ica" "${TrainingData}" "${FixThresh}" -m -h "${hp}") else - #-h is actually a subargument to -m, and will cause problems if specified without (or even not directly following) -m - fix_cmd=("${FSL_FIXDIR}/fix" "${fmrihp}.ica" "${TrainingData}" "${FixThresh}") + old_or_new_fix=$(determine_old_or_new_fix) + if [[ "${old_or_new_fix}" == "NEW" ]]; then + # In the "NEW" (1.06.12 or later) version of FIX, -h is no longer a subargument to -m, and can occur by itself + fix_cmd=("${FSL_FIXDIR}/fix" "${fmrihp}.ica" "${TrainingData}" "${FixThresh}" -h "${hp}") + + elif (( hp < 0 )); then + # Ok to proceed regardless of FIX version, since not filtering, so we don't need the filtering of the CIFTI + # that occurs in fix_3_clean. Simply don't supply either the -m or -h flags. + fix_cmd=("${FSL_FIXDIR}/fix" "${fmrihp}.ica" "${TrainingData}" "${FixThresh}") + + else # "OLD" and (hp >= 0) + # Already aborted in this case earlier, but include again here for completeness + log_Err_Abort "Due to a restriction in the FIX argument parsing, do_motion_regression=$3 is not supported for FIX versions prior to 1.06.12" + fi fi log_Msg "fix_cmd: ${fix_cmd[*]}" ## MPH: The 'fix' script itself will continue to log to its own custom files From f441da19d6d0fb25c02508bfcbbe64067ca6b119 Mon Sep 17 00:00:00 2001 From: Michael Harms Date: Fri, 8 Nov 2019 09:32:14 -0600 Subject: [PATCH 2/4] Add report of specific FIX version to other ICAFIX scripts --- ICAFIX/ReApplyFixMultiRunPipeline.sh | 12 +++++++++--- ICAFIX/ReApplyFixPipeline.sh | 7 +++++++ ICAFIX/hcp_fix | 2 +- ICAFIX/hcp_fix_multi_run | 8 +++++++- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/ICAFIX/ReApplyFixMultiRunPipeline.sh b/ICAFIX/ReApplyFixMultiRunPipeline.sh index 7515bd0e2..5e1f4780f 100755 --- a/ICAFIX/ReApplyFixMultiRunPipeline.sh +++ b/ICAFIX/ReApplyFixMultiRunPipeline.sh @@ -366,6 +366,12 @@ show_tool_versions() fsl_version_get fsl_ver log_Msg "FSL version: ${fsl_ver}" + # Show specific FIX version, if available + if [ -f ${FSL_FIXDIR}/fixversion ]; then + fixversion=$(cat ${FSL_FIXDIR}/fixversion ) + log_Msg "FIX version: $fixversion" + fi + old_or_new_version=$(determine_old_or_new_fsl ${fsl_ver}) if [ "${old_or_new_version}" == "OLD" ] ; then log_Err_Abort "FSL version 6.0.1 or greater is required." @@ -415,9 +421,6 @@ main() local this_script_dir=$(dirname "$0") fi - # Show tool versions - show_tool_versions - log_Msg "Starting main functionality" # Retrieve positional parameters @@ -1105,6 +1108,9 @@ log_Check_Env_Var CARET7DIR log_Check_Env_Var FSLDIR log_Check_Env_Var FSL_FIXDIR +# Show tool versions +show_tool_versions + # Determine whether named or positional parameters are used and invoke 'main' function if [[ ${1} == --* ]]; then # Named parameters (e.g. --parameter-name=parameter-value) are used diff --git a/ICAFIX/ReApplyFixPipeline.sh b/ICAFIX/ReApplyFixPipeline.sh index 9dadece8e..4eb4dedc9 100755 --- a/ICAFIX/ReApplyFixPipeline.sh +++ b/ICAFIX/ReApplyFixPipeline.sh @@ -268,6 +268,13 @@ show_tool_versions() log_Msg "Showing FSL version" fsl_version_get fsl_ver log_Msg "FSL version: ${fsl_ver}" + + # Show specific FIX version, if available + if [ -f ${FSL_FIXDIR}/fixversion ]; then + fixversion=$(cat ${FSL_FIXDIR}/fixversion ) + log_Msg "FIX version: $fixversion" + fi + } # ------------------------------------------------------------------------------ diff --git a/ICAFIX/hcp_fix b/ICAFIX/hcp_fix index d372deb53..6051acb4f 100755 --- a/ICAFIX/hcp_fix +++ b/ICAFIX/hcp_fix @@ -397,8 +397,8 @@ fi log_Msg "using training data file: ${TrainingData}" # set up fix command +# use array for whitespace safety, even if the rest of the script isn't if [[ ${doMotionRegression} == "TRUE" ]]; then - #use array for whitespace safety, even if the rest of the script isn't fix_cmd=("${FSL_FIXDIR}/fix" "${fmrihp}.ica" "${TrainingData}" "${FixThresh}" -m -h "${hp}") else old_or_new_fix=$(determine_old_or_new_fix) diff --git a/ICAFIX/hcp_fix_multi_run b/ICAFIX/hcp_fix_multi_run index af65d866d..cc6a8ffc8 100755 --- a/ICAFIX/hcp_fix_multi_run +++ b/ICAFIX/hcp_fix_multi_run @@ -140,6 +140,12 @@ log_Msg "Showing FSL version" fsl_version_get fsl_ver log_Msg "FSL version: ${fsl_ver}" +# Show specific FIX version, if available +if [ -f ${FSL_FIXDIR}/fixversion ]; then + fixversion=$(cat ${FSL_FIXDIR}/fixversion ) + log_Msg "FIX version: $fixversion" +fi + # Log FSL_FIX_MATLAB_MODE (from the settings.sh file) log_Msg "FSL_FIX_MATLAB_MODE: ${FSL_FIX_MATLAB_MODE}" @@ -738,8 +744,8 @@ fi log_Msg "using training data file: ${TrainingData}" # set up fix command +# use array for whitespace safety, even if the rest of the script isn't if [[ ${doMotionRegression} == "TRUE" ]]; then - #use array for whitespace safety, even if the rest of the script isn't fix_cmd=("${FSL_FIXDIR}/fix" "${concatfmrihp}.ica" "${TrainingData}" "${FixThresh}" -m -h "${AlreadyHP}") else #-h is actually a subargument to -m, and will cause problems if specified without (or even not directly following) -m From 503dc8a513e7807f9daad3adbc80ea70404ee16e Mon Sep 17 00:00:00 2001 From: Michael Harms Date: Fri, 8 Nov 2019 10:11:50 -0600 Subject: [PATCH 3/4] Update some of the fix-related comments to reflect changes as of fix 1.06.12 --- ICAFIX/ReApplyFixMultiRunPipeline.sh | 3 +++ ICAFIX/ReApplyFixPipeline.sh | 3 +++ ICAFIX/hcp_fix | 6 +++++- ICAFIX/hcp_fix_multi_run | 3 +++ 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/ICAFIX/ReApplyFixMultiRunPipeline.sh b/ICAFIX/ReApplyFixMultiRunPipeline.sh index 5e1f4780f..302968156 100755 --- a/ICAFIX/ReApplyFixMultiRunPipeline.sh +++ b/ICAFIX/ReApplyFixMultiRunPipeline.sh @@ -533,6 +533,9 @@ main() # It is for that reason that the code below needs to use separate calls to fix_3_clean, with and without DoVol # as an argument, rather than simply passing in the value of DoVol as set within this script. # Not sure if/when this non-intuitive behavior of fix_3_clean will change, but this is accurate as of fix1.067 + # UPDATE (11/8/2019): As of FIX 1.06.12, fix_3_clean interprets its 5th argument ("DoVol") in the usual boolean + # manner. However, since we already had a work-around to this problem, we will leave the code unchanged so that + # we don't need to add a FIX version dependency to the script. log_Msg "Use fixlist=$fixlist" diff --git a/ICAFIX/ReApplyFixPipeline.sh b/ICAFIX/ReApplyFixPipeline.sh index 4eb4dedc9..04121bf57 100755 --- a/ICAFIX/ReApplyFixPipeline.sh +++ b/ICAFIX/ReApplyFixPipeline.sh @@ -430,6 +430,9 @@ main() # It is for that reason that the code below needs to use separate calls to fix_3_clean, with and without DoVol # as an argument, rather than simply passing in the value of DoVol as set within this script. # Not sure if/when this non-intuitive behavior of fix_3_clean will change, but this is accurate as of fix1.067 + # UPDATE (11/8/2019): As of FIX 1.06.12, fix_3_clean interprets its 5th argument ("DoVol") in the usual boolean + # manner. However, since we already had a work-around to this problem, we will leave the code unchanged so that + # we don't need to add a FIX version dependency to the script. log_Msg "Use fixlist=$fixlist" diff --git a/ICAFIX/hcp_fix b/ICAFIX/hcp_fix index 6051acb4f..8e3d03d8f 100755 --- a/ICAFIX/hcp_fix +++ b/ICAFIX/hcp_fix @@ -69,10 +69,14 @@ fi ############################################################# -# Need at least fix version 1.06.12 for proper handling of the do_motion_regression=FALSE condition +# Function to return whether FIX version is "NEW" (i.e., 1.06.12 or later). +# Need at least that fix version for proper handling of the +# do_motion_regression=FALSE condition (when highpass>=0) + # NOTE: # Don't echo anything in this function other than the last echo # that outputs the return value + determine_old_or_new_fix() { # Default to "OLD", and change to "NEW" only if version is 1.06.12 or later diff --git a/ICAFIX/hcp_fix_multi_run b/ICAFIX/hcp_fix_multi_run index cc6a8ffc8..a29bc3ab5 100755 --- a/ICAFIX/hcp_fix_multi_run +++ b/ICAFIX/hcp_fix_multi_run @@ -749,6 +749,9 @@ if [[ ${doMotionRegression} == "TRUE" ]]; then fix_cmd=("${FSL_FIXDIR}/fix" "${concatfmrihp}.ica" "${TrainingData}" "${FixThresh}" -m -h "${AlreadyHP}") else #-h is actually a subargument to -m, and will cause problems if specified without (or even not directly following) -m + # Update (11/8/2019): No longer true as of FIX 1.06.12, but since filtering of the CIFTI in MR-FIX occurs in + # functionhighpassandvariancenormalize it doesn't matter, so we leave the code unchanged so that we don't + # need to add a FIX version dependency to the script fix_cmd=("${FSL_FIXDIR}/fix" "${concatfmrihp}.ica" "${TrainingData}" "${FixThresh}") fi log_Msg "fix_cmd: ${fix_cmd[*]}" From 6ca4875b0b361f7399a3b0e5854e4cab082c549b Mon Sep 17 00:00:00 2001 From: Michael Harms Date: Sat, 9 Nov 2019 09:35:23 -0600 Subject: [PATCH 4/4] Log all FIX parameters --- ICAFIX/hcp_fix | 50 ++++++++++++++++++---------- ICAFIX/hcp_fix_multi_run | 72 ++++++++++++++++++++++++---------------- 2 files changed, 77 insertions(+), 45 deletions(-) diff --git a/ICAFIX/hcp_fix b/ICAFIX/hcp_fix index 8e3d03d8f..dcf47b0b0 100755 --- a/ICAFIX/hcp_fix +++ b/ICAFIX/hcp_fix @@ -69,6 +69,8 @@ fi ############################################################# +### Support Functions + # Function to return whether FIX version is "NEW" (i.e., 1.06.12 or later). # Need at least that fix version for proper handling of the # do_motion_regression=FALSE condition (when highpass>=0) @@ -76,7 +78,7 @@ fi # NOTE: # Don't echo anything in this function other than the last echo # that outputs the return value - +# determine_old_or_new_fix() { # Default to "OLD", and change to "NEW" only if version is 1.06.12 or later @@ -120,6 +122,21 @@ determine_old_or_new_fix() echo ${old_or_new} } +function interpret_as_bool() +{ + case $(echo "$1" | tr '[:upper:]' '[:lower:]') in + (true | yes | 1) + echo TRUE + ;; + (false | no | none | 0) + echo FALSE + ;; + (*) + log_Err_Abort "error: '$1' is not valid for this argument, please use TRUE or FALSE" + ;; + esac +} + ############################################################# # Set global variables @@ -170,6 +187,10 @@ log_Msg "FSL_FIX_MATLAB_MODE: ${FSL_FIX_MATLAB_MODE}" ############################################################# +## --------------------------------------------------------------------------- +## Parse and check parameters +## --------------------------------------------------------------------------- + fmri=$1 cd `dirname $fmri` fmri=`basename $fmri` # After this, $fmri no longer includes the leading directory components @@ -180,21 +201,6 @@ fi hp=$2 -function interpret_as_bool() -{ - case $(echo "$1" | tr '[:upper:]' '[:lower:]') in - (true | yes | 1) - echo TRUE - ;; - (false | no | none | 0) - echo FALSE - ;; - (*) - log_Err_Abort "error: '$1' is not valid for this argument, please use TRUE or FALSE" - ;; - esac -} - doMotionRegression=$(interpret_as_bool "$3") # do_motion_regression=FALSE combined with hp filtering doesn't work properly with single-run FIX, # for fix versions earlier than 1.06.12, so don't allow that mode of operation if the fix version @@ -224,13 +230,23 @@ if [ -z "${FixThresh}" ]; then FixThresh=10 fi +## --------------------------------------------------------------------------- +## Report parameters +## --------------------------------------------------------------------------- + +log_Msg "fMRI Name: ${fmri}" +log_Msg "highpass: ${hp}" +log_Msg "doMotionRegression: ${doMotionRegression}" +log_Msg "TrainingData: ${TrainingData}" +log_Msg "FixThresh: ${FixThresh}" +log_Msg "DeleteIntermediates: ${DeleteIntermediates}" + ## --------------------------------------------------------------------------- ## Preparation (highpass) ## --------------------------------------------------------------------------- tr=`$FSLDIR/bin/fslval $fmri pixdim4` log_Msg "tr: ${tr}" -log_Msg "processing FMRI file ${fmri} with highpass ${hp}" fmrihp=${fmri}_hp${hp} if (( hp > 0 )); then diff --git a/ICAFIX/hcp_fix_multi_run b/ICAFIX/hcp_fix_multi_run index a29bc3ab5..d1d371156 100755 --- a/ICAFIX/hcp_fix_multi_run +++ b/ICAFIX/hcp_fix_multi_run @@ -156,7 +156,8 @@ FSL_FIX_WBC=${Caret7_Command} # Used by fix_3_clean in interpreted matlab/octav ############################################################# -# +### Support Functions + # NOTE: # Don't echo anything in this function other than the last echo # that outputs the return value @@ -247,18 +248,40 @@ demeanMovementRegressors() { echo "${AllOut}" > ${Out} } -fsl_version_get fsl_ver -log_Msg "FSL version: ${fsl_ver}" +function interpret_as_bool() +{ + case $(echo "$1" | tr '[:upper:]' '[:lower:]') in + (true | yes | 1) + echo TRUE + ;; + (false | no | none | 0) + echo FALSE + ;; + (*) + log_Err_Abort "error: '$1' is not valid for this argument, please use TRUE or FALSE" + ;; + esac +} -old_or_new_version=$(determine_old_or_new_fsl ${fsl_ver}) +############################################################# + +## --------------------------------------------------------------------------- +## Check whether FSL version is recent enough +## --------------------------------------------------------------------------- +old_or_new_version=$(determine_old_or_new_fsl ${fsl_ver}) if [ "${old_or_new_version}" == "OLD" ] ; then log_Err_Abort "FSL version 6.0.1 or greater is required." fi +## --------------------------------------------------------------------------- +## Parse and check parameters +## --------------------------------------------------------------------------- + +fmris=`echo ${1} | sed 's/@/ /g'` # replaces the @ that combines the filenames with ' ' + hp=$2 if [[ "${hp}" == "0" ]]; then - log_Msg "hp=0 corresponds to a linear detrend" pdFlag=TRUE fi if [[ "${hp}" == pd* ]]; then @@ -277,27 +300,7 @@ if [[ $(echo "${hpNum} < 0" | bc) == "1" ]]; then #Logic of this script does no log_Err_Abort "highpass value must not be negative" fi -log_Msg "hp: ${hp}" - -fmris=`echo ${1} | sed 's/@/ /g'` # replaces the @ that combines the filenames with ' ' -log_Msg "fmris: ${fmris}" ConcatName="${3}" -log_Msg "ConcatName: ${ConcatName}" - -function interpret_as_bool() -{ - case $(echo "$1" | tr '[:upper:]' '[:lower:]') in - (true | yes | 1) - echo TRUE - ;; - (false | no | none | 0) - echo FALSE - ;; - (*) - log_Err_Abort "error: '$1' is not valid for this argument, please use TRUE or FALSE" - ;; - esac -} doMotionRegression=$(interpret_as_bool "$4") @@ -316,10 +319,23 @@ if [ -z "${FixThresh}" ]; then FixThresh=10 fi -DIR=`pwd` -log_Msg "PWD : $DIR" +## --------------------------------------------------------------------------- +## Report parameters +## --------------------------------------------------------------------------- -#echo $fmris | tr ' ' '\n' #separates paths separated by ' ' +log_Msg "fMRI Names: ${fmris}" +log_Msg "highpass: ${hp}" +if [[ "${hp}" == "0" ]]; then + log_Msg "hp=0 corresponds to a linear detrend" +fi +log_Msg "ConcatName: ${ConcatName}" +log_Msg "doMotionRegression: ${doMotionRegression}" +log_Msg "TrainingData: ${TrainingData}" +log_Msg "FixThresh: ${FixThresh}" +log_Msg "DeleteIntermediates: ${DeleteIntermediates}" + +DIR=`pwd` +log_Msg "PWD: $DIR" ## --------------------------------------------------------------------------- ## Preparation (highpass) on the individual runs