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

HCP HippUnfold and Post HippUnfold Implementations #320

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
94 changes: 94 additions & 0 deletions Examples/Scripts/RunHippUnfoldHCP.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
#!/bin/bash

get_batch_options() {
local arguments=("$@")

command_line_specified_study_folder=""
command_line_specified_subject=""
command_line_specified_run_local="FALSE"

local index=0
local numArgs=${#arguments[@]}
local argument

while [ ${index} -lt ${numArgs} ]; do
argument=${arguments[index]}

case ${argument} in
--StudyFolder=*)
command_line_specified_study_folder=${argument#*=}
index=$(( index + 1 ))
;;
--Subject=*)
command_line_specified_subject=${argument#*=}
index=$(( index + 1 ))
;;
--runlocal)
command_line_specified_run_local="TRUE"
index=$(( index + 1 ))
;;
*)
echo ""
echo "ERROR: Unrecognized Option: ${argument}"
echo ""
exit 1
;;
esac
done
}

get_batch_options "$@"

StudyFolder="${HOME}/projects/Pipelines_ExampleData" #Location of Subject folders (named by SubjectID)
Subjlist="100307 100610" #Space delimited list of subject IDs
EnvironmentScript="${HOME}/projects/Pipelines/Examples/Scripts/SetUpHCPPipeline.sh" #Pipeline environment script

if [ -n "${command_line_specified_study_folder}" ]; then
StudyFolder="${command_line_specified_study_folder}"
fi

if [ -n "${command_line_specified_subject}" ]; then
Subjlist="${command_line_specified_subject}"
fi

#Set up pipeline environment variables and software
source "$EnvironmentScript"

# Log the originating call
echo "$@"

#NOTE: syntax for QUEUE has changed compared to earlier pipeline releases,
#DO NOT include "-q " at the beginning
#default to no queue, implying run local
QUEUE=""
#QUEUE="hcp_priority.q"

########################################## INPUTS ##########################################

#Scripts called by this script do assume they run on the outputs of the PostFreeSurfer Pipeline

######################################### DO WORK ##########################################

for Subject in $Subjlist ; do
echo $Subject

if [[ "${command_line_specified_run_local}" == "TRUE" || "$QUEUE" == "" ]] ; then
echo "About to locally run ${HCPPIPEDIR}/HippUnfoldHCP/HippUnfoldHCP.sh"
queuing_command=("$HCPPIPEDIR"/global/scripts/captureoutput.sh)
else
echo "About to use fsl_sub to queue ${HCPPIPEDIR}/HippUnfoldHCP/HippUnfoldHCP.sh"
queuing_command=("$FSLDIR/bin/fsl_sub" -q "$QUEUE")
fi

"${queuing_command[@]}" "$HCPPIPEDIR"/HippUnfoldHCP/HippUnfoldHCP.sh \
--study-folder="$StudyFolder" \
--subject="$Subject" \

# The following lines are used for interactive debugging to set the positional parameters: $1 $2 $3 ...

echo "set -- --study-folder=$StudyFolder \
--subject=$Subject" \

echo ". ${EnvironmentScript}"

done
94 changes: 94 additions & 0 deletions Examples/Scripts/RunPostHippUnfoldHCP.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
#!/bin/bash

get_batch_options() {
local arguments=("$@")

command_line_specified_study_folder=""
command_line_specified_subject=""
command_line_specified_run_local="FALSE"

local index=0
local numArgs=${#arguments[@]}
local argument

while [ ${index} -lt ${numArgs} ]; do
argument=${arguments[index]}

case ${argument} in
--StudyFolder=*)
command_line_specified_study_folder=${argument#*=}
index=$(( index + 1 ))
;;
--Subject=*)
command_line_specified_subject=${argument#*=}
index=$(( index + 1 ))
;;
--runlocal)
command_line_specified_run_local="TRUE"
index=$(( index + 1 ))
;;
*)
echo ""
echo "ERROR: Unrecognized Option: ${argument}"
echo ""
exit 1
;;
esac
done
}

get_batch_options "$@"

StudyFolder="${HOME}/projects/Pipelines_ExampleData" #Location of Subject folders (named by SubjectID)
Subjlist="100307 100610" #Space delimited list of subject IDs
EnvironmentScript="${HOME}/projects/Pipelines/Examples/Scripts/SetUpHCPPipeline.sh" #Pipeline environment script

if [ -n "${command_line_specified_study_folder}" ]; then
StudyFolder="${command_line_specified_study_folder}"
fi

if [ -n "${command_line_specified_subject}" ]; then
Subjlist="${command_line_specified_subject}"
fi

#Set up pipeline environment variables and software
source "$EnvironmentScript"

# Log the originating call
echo "$@"

#NOTE: syntax for QUEUE has changed compared to earlier pipeline releases,
#DO NOT include "-q " at the beginning
#default to no queue, implying run local
QUEUE=""
#QUEUE="hcp_priority.q"

########################################## INPUTS ##########################################

#Scripts called by this script do assume they run on the outputs of the PostFreeSurfer Pipeline

######################################### DO WORK ##########################################

for Subject in $Subjlist ; do
echo $Subject

if [[ "${command_line_specified_run_local}" == "TRUE" || "$QUEUE" == "" ]] ; then
echo "About to locally run ${HCPPIPEDIR}/HippUnfoldHCP/PostHippUnfoldHCP.sh"
queuing_command=("$HCPPIPEDIR"/global/scripts/captureoutput.sh)
else
echo "About to use fsl_sub to queue ${HCPPIPEDIR}/HippUnfoldHCP/PostHippUnfoldHCP.sh"
queuing_command=("$FSLDIR/bin/fsl_sub" -q "$QUEUE")
fi

"${queuing_command[@]}" "$HCPPIPEDIR"/HippUnfoldHCP/PostHippUnfoldHCP.sh \
--study-folder="$StudyFolder" \
--subject="$Subject" \

# The following lines are used for interactive debugging to set the positional parameters: $1 $2 $3 ...

echo "set -- --study-folder=$StudyFolder \
--subject=$Subject" \

echo ". ${EnvironmentScript}"

done
2 changes: 2 additions & 0 deletions Examples/Scripts/SetUpHCPPipeline.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export MATLAB_COMPILER_RUNTIME=/export/matlab/MCR/R2017b/v93
# If a suitable version of wb_command is on your $PATH, CARET7DIR can be blank
export CARET7DIR=
export HCPCIFTIRWDIR="$HCPPIPEDIR"/global/matlab/cifti-matlab
export HIPPUNFOLDPATH="${HOME}/pipeline_tools/HippUnfold/khanlab_hippunfold_latest.sif"
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a highly specific local path. Do you have any conventions about where users should put external tools that are required to run certain HCP pipelines? For example, $HCPPIPEDIR/external.

Copy link
Member

@coalsont coalsont Jan 8, 2025

Choose a reason for hiding this comment

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

Users should not add anything inside of the repo folder. That way madness lies.

More to the point, the pipelines setup instructions don't tell users where to install FSL, or freesurfer, or MSM, etc, and I don't think we should start doing so. The standard "set this variable to where you have downloaded hippunfold" formula should apply.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the standard convention we have used for all of the HCP Pipelines so it is odd to be complaining about it here. It is intended to be changed but is not supposed to be blank (unlike the CARET7DIR).

export HIPPUNFOLD_CACHE_DIR="${HOME}/.cache/hippunfold"

## Set up FSL (if not already done so in the running environment)
## Uncomment the following 2 lines (remove the leading #) and correct the FSLDIR setting for your setup
Expand Down
101 changes: 101 additions & 0 deletions HippUnfoldHCP/HippUnfoldHCP.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#!/bin/bash
set -eu
pipedirguessed=0
if [[ "${HCPPIPEDIR:-}" == "" ]]
then
# pipedirguessed=1
#fix this if the script is more than one level below HCPPIPEDIR
export HCPPIPEDIR="$(dirname -- "$0")/.."
fi

source "$HCPPIPEDIR/global/scripts/newopts.shlib" "$@"
source "$HCPPIPEDIR/global/scripts/debug.shlib" "$@"


opts_SetScriptDescription "Make some BIDS structures and run HippUnfold"

opts_AddMandatory '--study-folder' 'StudyFolder' 'path' "folder containing all subjects"
opts_AddMandatory '--subject' 'Subject' 'subject ID' ""
Copy link
Contributor

Choose a reason for hiding this comment

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

This was changed to Session during the refactoring?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as the other comment.

opts_AddOptional '--hippunfold-dir' 'HippUnfoldDIR' 'path' "location of HippUnfold outputs"

opts_ParseArguments "$@"

if ((pipedirguessed))
then
log_Err_Abort "HCPPIPEDIR is not set, you must first source your edited copy of Examples/Scripts/SetUpHCPPipeline.sh"
fi

opts_ShowValues

T1wFolder="$StudyFolder/$Subject/T1w" # input data

if [ -z ${HippUnfoldDIR} ] ; then
HippUnfoldDIR="${T1wFolder}/HippUnfold"
fi

#Couldn't get non-BIDS to work (wouldn't do anything)
#HippUnfoldT1wFolder="$HippUnfoldDIR/T1w"
#HippUnfoldT2wFolder="$HippUnfoldDIR/T2w"
#HippUnfoldT2wFolder="$HippUnfoldDIR/T1wT2w"

HippUnfoldT1wFolder="$HippUnfoldDIR/T1w/sub-${Subject}/anat"
HippUnfoldT2wFolder="$HippUnfoldDIR/T2w/sub-${Subject}/anat"
HippUnfoldT1wT2wFolder="$HippUnfoldDIR/T1wT2w/sub-${Subject}/anat"

HippUnfoldT1wFolderOut="$HippUnfoldDIR/T1w_hippunfold"
HippUnfoldT2wFolderOut="$HippUnfoldDIR/T2w_hippunfold"
HippUnfoldT1wT2wFolderOut="$HippUnfoldDIR/T1wT2w_hippunfold"


T1wImage="$T1wFolder/T1w_acpc_dc_restore.nii.gz"
T2wImage="$T1wFolder/T2w_acpc_dc_restore.nii.gz"


if [ ! -f "$T1wImage" ]; then
echo "Error: T1w image not found at $T1wImage" >&2
exit 1
fi

if [ ! -f "$T2wImage" ]; then
echo "Error: T2w image not found at $T2wImage" >&2
exit 1
fi


mkdir -p "$HippUnfoldT1wFolder" "$HippUnfoldT2wFolder" "$HippUnfoldT1wT2wFolder"

#Couldn't get non-BIDS to work (wouldn't do anything)
#cp "$T1wImage" "$HippUnfoldT1wFolder/${Subject}_T1w_acpc_dc_restore.nii.gz"
#cp "$T2wImage" "$HippUnfoldT1wFolder/${Subject}_T2w_acpc_dc_restore.nii.gz"
#cp "$T1wImage" "$HippUnfoldT2wFolder/${Subject}_T1w_acpc_dc_restore.nii.gz"
#cp "$T2wImage" "$HippUnfoldT2wFolder/${Subject}_T2w_acpc_dc_restore.nii.gz"
#cp "$T1wImage" "$HippUnfoldT1wT2wFolder/${Subject}_T1w_acpc_dc_restore.nii.gz"
#cp "$T2wImage" "$HippUnfoldT1wT2wFolder/${Subject}_T2w_acpc_dc_restore.nii.gz"

cp "$T1wImage" "$HippUnfoldT1wFolder/sub-${Subject}_T1w.nii.gz"
cp "$T2wImage" "$HippUnfoldT1wFolder/sub-${Subject}_T2w.nii.gz"
cp "$T1wImage" "$HippUnfoldT2wFolder/sub-${Subject}_T1w.nii.gz"
cp "$T2wImage" "$HippUnfoldT2wFolder/sub-${Subject}_T2w.nii.gz"
cp "$T1wImage" "$HippUnfoldT1wT2wFolder/sub-${Subject}_T1w.nii.gz"
cp "$T2wImage" "$HippUnfoldT1wT2wFolder/sub-${Subject}_T2w.nii.gz"
Comment on lines +75 to +80
Copy link
Member

Choose a reason for hiding this comment

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

Does it really need to create T1w/T2w.nii.gz and T2w/T1w.nii.gz? Do the 3 calls even need 3 separate input folders (with currently the same initial contents)? Why are there 3 hippunfold calls anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is all necessary for the BIDS wrapping, if we can get working direct input paths, I would not do any of this. There are 3 HippUnfold calls because there are 3 ways to call HippUnfold currently. I am hopeful we could standardize on using both the T1w and T2w in HippUnfold 2.0, but we need to hear from the HippUnfold folks that it is better or at least not worse than using individual inputs.



log_Msg "Created folder structure under $HippUnfoldDIR and copied T1w and T2w images"
log_Msg "Starting HippUnfold pipeline for subject: $Subject"

#Couldn't get non-BIDS to work (wouldn't do anything) #Seriously: don't put a $ here...
#apptainer run --bind $StudyFolder -e $HIPPUNFOLDPATH $HippUnfoldT1wFolder $HippUnfoldT1wFolder participant --modality T1w --path-T1w $HippUnfoldT1wFolder/{Subject}_T1w_acpc_dc_restore.nii.gz --cores all --force-output --generate_myelin_map
#apptainer run --bind $StudyFolder -e $HIPPUNFOLDPATH $HippUnfoldT2wFolder $HippUnfoldT2wFolder participant --modality T2w --path-T2w $HippUnfoldT2wFolder/{Subject}_T2w_acpc_dc_restore.nii.gz --cores all --force-output --generate_myelin_map
#apptainer run --bind $StudyFolder -e $HIPPUNFOLDPATH $HippUnfoldT2wFolder $HippUnfoldT2wFolder participant --modality T2w --path-T2w $HippUnfoldT2wFolder/{Subject}_T2w_acpc_dc_restore.nii.gz --cores all --force-output --generate_myelin_map --force-nnunet-model T1T2w

log_Msg "Running T1w HippUnfold for subject: $Subject"
apptainer run --bind $StudyFolder -e $HIPPUNFOLDPATH $HippUnfoldT1wFolder $HippUnfoldT1wFolderOut participant --modality T1w --cores all --force-output --generate_myelin_map --output-density 0p5mm 1mm 2mm
log_Msg "T1w HippUnfold completed."
log_Msg "Running T2w HippUnfold for subject: $Subject"
apptainer run --bind $StudyFolder -e $HIPPUNFOLDPATH $HippUnfoldT2wFolder $HippUnfoldT2wFolderOut participant --modality T2w --cores all --force-output --generate_myelin_map --output-density 0p5mm 1mm 2mm
log_Msg "T2w HippUnfold completed."
log_Msg "Running T1wT2w HippUnfold for subject: $Subject"
apptainer run --bind $StudyFolder -e $HIPPUNFOLDPATH $HippUnfoldT1wT2wFolder $HippUnfoldT1wT2wFolderOut participant --modality T1w --cores all --force-output --generate_myelin_map --output-density 0p5mm 1mm 2mm --force-nnunet-model T1T2w
log_Msg "T1wT2w HippUnfold completed."
Copy link
Contributor

Choose a reason for hiding this comment

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

I knew this day would come sooner or later. This will force us to run containers inside containers (HippUnfold inside QuNex), on paper it should work. I guess we will see in practice if there are any issues with this.

What we need to figure out is whether we should put the HippUnfold container inside the QuNex container (this adds a couple GBs to the QuNex size) or if we require users to download it manually. I am leaning towards the first option as it is more user friendly and nowadays size is not that important as we all have good connectivity and enough disk space. A case for having the container outside could be made by saying this is a pipeline that will not be executed by most of our users?

Need to think this through a bit.

Copy link
Member

Choose a reason for hiding this comment

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

Do they provide their code such that it can be run without their container wrapping it? I think of containers as a convenience of not having to manually configure a bunch of software, not as a replacement for being able to install someone's software.

Choose a reason for hiding this comment

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

HippUnfold v2.0.0 will be available on conda (ETA ~1 month)
The current HippUnfold v1.5.2 can be installed with git clone; pip install -e ., however it also uses containers internally for dependencies like nighres, wb_command, etc. See here (and note that poetry is not necessary, its just for python linting)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the info. I felt like adding your whole container into our container would be suboptimal space wise as there should be some overlap between the tools we depend on. We indeed both use wb_command, so that one would be redundant for sure, there could be others. I will first try to do this optimally and install all the dependencies manually. If this does not work and is too much of a hassle, we can go the container route.

@glasserm What is your preferred timeline here? Would you like to wait for HippUnfold v2.0.0 and then start processing with this one. Sounds like a major upgrade, that is only a month or so away. Or is this urgent and waiting an extra month is not an option?

Choose a reason for hiding this comment

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

RE: #Couldn't get non-BIDS to work (wouldn't do anything) #Seriously: don't put a $ here...

The Subject wildcard should simply be subject. This can be used to avoid having to make a BIDS directory as input. Note that hippunfold expects to iterate over all possible completions of the wildcard subject (and can also iterate over other wildcards like session or acq, as per pyBIDS). Best parallelization can be achieved by running batches of subjects, but I udnerstand you may not want to set up your pipelines that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the info. I felt like adding your whole container into our container would be suboptimal space wise as there should be some overlap between the tools we depend on. We indeed both use wb_command, so that one would be redundant for sure, there could be others. I will first try to do this optimally and install all the dependencies manually. If this does not work and is too much of a hassle, we can go the container route.

@glasserm What is your preferred timeline here? Would you like to wait for HippUnfold v2.0.0 and then start processing with this one. Sounds like a major upgrade, that is only a month or so away. Or is this urgent and waiting an extra month is not an option?

This isn't intended to be merged until HippUnfold v2.0.0 is available and the PR reflects and changes needed for that. I agree that ideally you would install HippUnfold + dependencies in QuNex rather than the container.

Copy link
Contributor Author

@glasserm glasserm Jan 11, 2025

Choose a reason for hiding this comment

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

RE: #Couldn't get non-BIDS to work (wouldn't do anything) #Seriously: don't put a $ here...

The Subject wildcard should simply be subject. This can be used to avoid having to make a BIDS directory as input. Note that hippunfold expects to iterate over all possible completions of the wildcard subject (and can also iterate over other wildcards like session or acq, as per pyBIDS). Best parallelization can be achieved by running batches of subjects, but I understand you may not want to set up your pipelines that way.

I currently wrap in some BIDS because I couldn't get the non-BIDS options to work reliably. If you guys provide an ability to simply specify paths to relevant inputs, I will remove the wrapped BIDS.


log_Msg "HippUnfold pipeline completed successfully for subject: $Subject"
Loading