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.

Sign up for GitHub

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 10 commits into
base: master
Choose a base branch
from
Open

HCP HippUnfold and Post HippUnfold Implementations #320

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a50375f
*Initial Commit of HippUnfoldHCP and PostHippUnfoldHCP, tested but pr…
glasserm Dec 8, 2024
8d8c3ca
*Add Apptainer/Singularity Cache control
glasserm Dec 14, 2024
b279be6
*Make vertex areas specific to the volume space they are located in
glasserm Dec 14, 2024
30356e7
*Combine hipp and dentate in CIFTI files
glasserm Dec 21, 2024
6a3f00b
*Remove matlab hack code
glasserm Dec 21, 2024
10238bf
*Add inner and outer surfaces
glasserm Jan 3, 2025
3d320b7
*Remove absolute paths
glasserm Jan 5, 2025
03a4bc6
*math doesn't require the Var to be used
glasserm Jan 11, 2025
8b1c465
*math allows Var to be used without modification.
glasserm Jan 11, 2025
544d4ba
remove functionalization of single loop body
coalsont Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions Examples/Scripts/RunHippUnfoldHCP.sh
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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"
coalsont marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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.

Copy link
Member

Choose a reason for hiding this comment

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

Matt says the T1w image in the T2w folder may be needed for registration. The T2w image in the T1w folder still seems strange to me. Another question, can these be symlinks instead of actual copies? It looks like the container calls bind mount the entire study folder, so I would think relative symlinks would work.



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.

Copy link

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?

Copy link

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 •
edited
Loading

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