Update HippUnfold HCP processing by orenpoliva · Pull Request #410 · Washington-University/HCPpipelines

orenpoliva · 2026-08-25T15:55:13Z

Main changes for RunHippUnfold, HippUnfold, PostHippUnfold, RunPostHippUnfold: 

-Updated the HippUnfold command to use the Maguire T2w model, including --force-nnunet-model maguire_T2w and inner/outer registration settings of 0. 

-Changed the output/file structure from supporting separate T1w, T2w, and T1wT2w models to using the Maguire T2w model. Because only one model is now used, the model-specific T1w, T2w, and T1wT2w subdirectories were removed. 

-Log files are now written to a dedicated logging directory within each subject's T1w/HippUnfold directory. 

-In HippUnfoldHCP.sh, imported runlocal from RunHippUnfoldHCP, and added separate per-job cache and Conda directories when runlocal=FALSE. This prevents simultaneous subjects from sharing these directories, which had caused failures when multiple HippUnfold jobs were run in parallel. These temporary per-job directories are removed after successful completion. 

-In PostHippUnfoldHCP.sh, one use of ${Scalar} in the dentate-label generation section was replaced with an explicit thickness.shape.gii input. ${Scalar} is likely a leftover from the preceding scalar-processing loop. The metric is used as the template for generating the constant-valued dentate label; the thickness values themselves are not used in the calculation. 

-Updated RunHippUnfoldHCP.sh and RunPostHippUnfoldHCP.sh for generic study-folder, subject-list, environment-script.

coalsont · 2026-08-26T03:11:02Z

+    if [[ "$RunLocal" == "FALSE" ]]
+    then
+    	# Separate cache and conda directory for each subject to avoid crash due to parallel use
+	JobCache="${HIPPUNFOLD_CACHE_DIR}/job-cache/${JOB_ID:-local}_${Subject}"


Maybe I should implement support for folders in tempfiles.shlib, so we wouldn't need this hack just to avoid collision on something we presumably don't need to preserve.

glasserm · 2026-08-31T00:48:22Z

-#!/bin/bash 
-


Please always include the shabang.

glasserm

Given the really extensive PostHippUnfold changes, I would like to review at test dataset.

orenpoliva · 2026-09-08T14:47:17Z

HippUnfold

HippUnfoldHCP.sh
Identified a cache cleanup problem: subject-specific job cache directories were not always being deleted, causing the cache directory to grow to over 400 GB. To address this, an EXIT trap was added to HippUnfoldHCP.sh so that the subject-specific job cache is removed whenever the script exits, including following an error.
Added a Snakemake unlock step to remove stale locks that may remain after incomplete or failed previous runs and prevent subsequent runs from proceeding.

Added the --latency-wait 60 and --rerun-incomplete flags to the HippUnfold command to improve robustness against common causes of failed or interrupted runs.

RunPostHippUnfoldHCP.sh
Added a check to verify that HippUnfold completed successfully for each subject before initiating post-processing. Subjects without a successfully completed HippUnfold run are excluded from post-processing, and the user is notified.
PostHippUnfoldHCP.sh
Changed the output directory name from native to Native.
Completed the spec-file merge section. Previously, merged cortical + hippocampal spec files were created only for native (T1w + MNI), 512 (T1w + MNI), and 2k (T1w only). The missing merges were added for the remaining meshes and spaces.

Potentially fixed a bug in the transformation of hippocampal surfaces from T1w/physical space to MNI space. The wb_command -surface-apply-warpfield command supplied an MRI image (T1w_restore.nii.gz) as the argument to -fnirt, rather than the corresponding FNIRT transformation field. The original command contained:
wb_command -surface-apply-warpfield

"${AtlasFolder}/xfms/standard2acpc_dc.nii.gz"

-fnirt "${AtlasFolder}/T1w_restore.nii.gz"
The -fnirt argument was changed to:
-fnirt "${AtlasFolder}/xfms/acpc_dc2standard.nii.gz"
Deleted and recreated all HippUnfold directories for the 45 subjects in both YA_HCP_Final and YA_HCP_ReTest_Final using the current version of the script. As a result, spec files generated by older versions of the script, which included additional NIfTI files such as T1w or T2w images without the _restore suffix, are no longer present.
PostHippUnfoldHCP.sh previously used two-space indentation. To conform to Tim's coding standard, the entire script was reformatted to use four-space indentation.

Questions:

Should QUEUE be defined in SetUpHCPPipeline.sh rather than directly in the run script?
In PostHippUnfoldHCP.sh, for the 2k, 8k, and 18k hippocampal meshes, I merged the hippocampal spec files with the 164k cortical MSMAll spec file. Is 164k the correct cortical spec file to use for all three hippocampal mesh resolutions?
Matt — you said that only the T1w and T2w NIfTI files should be included. I also see T1wDividedByT2w.nii.gz and T1wDividedByT2w_ribbon.nii.gz in the spec files. Should these also be removed?

coalsont · 2026-09-09T04:32:27Z

Should QUEUE be defined in SetUpHCPPipeline.sh rather than directly in the run script?

No. Different pipelines are (somewhat) expected to use different queues because they have different levels of internal parallelism and memory requirements.

coalsont · 2026-09-09T22:12:18Z

-      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}"
+    echo "$Subject"
+
+    LogDir="${StudyFolder}/${Subject}/T1w/HippUnfold/logs/HippUnfoldHCP"
+    mkdir -p "$LogDir"
+    cd "$LogDir"
+
+    if [[ "${command_line_specified_run_local}" == "TRUE" || "$QUEUE" == "" ]] ; then
+
+        echo "About to locally run ${HCPPIPEDIR}/HippUnfoldHCP/HippUnfoldHCP.sh"
+
+        if ! "$HCPPIPEDIR"/global/scripts/captureoutput.sh \
+            "$HCPPIPEDIR"/HippUnfoldHCP/HippUnfoldHCP.sh \
+            --study-folder="$StudyFolder" \
+            --subject="$Subject"
+        then
+            echo "ERROR: HippUnfold failed for subject: $Subject"
+            exit 1
+        fi
+


I'm not sure we want local runs to stop at the first failed subject, as it doesn't necessarily indicate that later subjects will also fail. I'd suggest changing all this back to the normal queueing_command pattern.

coalsont · 2026-09-09T22:18:42Z

+for Subject in $Subjlist ; do
+    HippLogDir="${StudyFolder}/${Subject}/T1w/HippUnfold/logs/HippUnfoldHCP"
+    LatestLog=$(ls -t "${HippLogDir}"/*.o* 2>/dev/null | head -1)
+
+    if [[ -n "$LatestLog" ]] && grep -qF "HippUnfold pipeline completed successfully for subject: ${Subject}" "$LatestLog" ; then
+        SuccessfulSubjects+=" ${Subject}"
+    else
+        FailedSubjects+=" ${Subject}"
+    fi
+done


I don't think we want to use log files to decide whether to try to run the Post pipeline, particularly not by checking for a highly specific and otherwise cosmetic message (that any developer might edit or remove without much thought). We could test for the existence of an important data file, but only if we really need the launcher to not even attempt some subjects. I think it is rare that we have a launcher try to do this at all, rather than simply trying every subject in the user-provided list.

Launcher code like that is typically something the users add after the fact for convenience.

coalsont · 2026-09-09T22:22:18Z

+    LogDir="${StudyFolder}/${Subject}/T1w/HippUnfold/logs/HippUnfoldHCP"
+    mkdir -p "$LogDir"
+    cd "$LogDir"


Putting the job logs into subject-specific folders is not how we usually do things, and makes it harder to grep all the logs from a batch at once.

If we don’t put the log files in the subject-specific directories, would it be a good idea to include the subject ID in the log filename?

I'm not sure that we can set the name of the logfile. The queuing system typically generates it from the name of the executable and the job number (a counter that never resets, even through reboots).

Let's treat logs as other pipelines do.

coalsont · 2026-09-09T22:25:10Z

+Subjlist="103818 105923 111312 114823 115320 122317 125525 130518 135528
+137128 139839 143325 144226 146129 149337 149741 151526 158035
+169343 172332 175439 177746 185442 187547 192439 194140 195041
+200109 200614 204521 250427 287248 341834 433839 562345 599671
+601127 627549 660951 662551 783462 859671 861456 877168 917255"


Please make a copy with a different name or path (for instance, appending your initials, or somewhere outside the repo entirely) and don't commit it, rather than committing your testing paths and subject list to the example.

After noticing I did this, I committed the scripts again with empty paths. Did you not receive the updated commit?

See my comment on line 113, you appear to have tried to replace the contents of the file, but left the entire old version on the end of it.

coalsont · 2026-09-09T22:32:07Z

+    fi
+}
+
+trap cleanup EXIT


If anyone tries to use tempfiles.shlib, this trap ... EXIT will prevent it from working. We can probably leave this as a "remove if completed successfully" rather than worrying about it. Or, I can add folder support to tempfiles.shlib, to also handle the collision avoidance without expecting JOB_ID to be a robust indicator.

orenpoliva · 2026-09-18T19:51:29Z

I uploaded a new version that addresses Tim’s comments. It also includes several additions that I thought would improve the scripts:

Added options for specifying the environment setup script (SetUpHCPPipeline.sh) and execution queue.
Per Tim’s request, moved the log files from the individual subject directories back to a common directory. I also added an option for specifying the log directory, which defaults to StudyFolder.
Added a cortical-mesh option to RunPostHippUnfold and PostHippUnfold. It accepts 32 for 3T data and 59 for 7T data, with 32 as the default. In PostHippUnfold, the selected cortical mesh is merged into the hippocampal spec files at each standard mesh resolution (512, 2k, 8k, and 18k). The native hippocampal mesh is instead merged with the native cortical mesh.

The pipeline output can be inspected for subject 103818 at:

/media/myelin/oren/HippUnfo

coalsont · 2026-09-19T04:37:46Z

2. I also added an option for specifying the log directory, which defaults to StudyFolder.

Please not StudyFolder. The usual default for queueing tools is to log to the current directory (.), which isn't amazing for organization if you are lazy, but can be controlled if you plan ahead and cd somewhere appropriate. I don't want the pipelines to specifically advocate for "spam your data directory with log files, in the same place as your subject folders". If the user doesn't specify a folder, the best idea is not to provide the log folder argument to the command (maybe fsl_sub has a configuration file option for this, it would be a shame to break it if so).

coalsont · 2026-09-19T04:40:58Z

+if [[ "$RunLocal" == "TRUE" || -z "$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" -l "$LogFolder")
+
+fi
+
+"${queuing_command[@]}" \


This is in a loop, it should be indented.

glasserm · 2026-09-19T20:17:27Z

I also added an option for specifying the log directory, which defaults to StudyFolder.

Please not StudyFolder. The usual default for queueing tools is to log to the current directory (.), which isn't amazing for organization if you are lazy, but can be controlled if you plan ahead and cd somewhere appropriate. I don't want the pipelines to specifically advocate for "spam your data directory with log files, in the same place as your subject folders". If the user doesn't specify a folder, the best idea is not to provide the log folder argument to the command (maybe fsl_sub has a configuration file option for this, it would be a shame to break it if so).

I agree.

glasserm · 2026-09-19T20:19:46Z

HippUnfold
1. HippUnfoldHCP.sh
   Identified a cache cleanup problem: subject-specific job cache directories were not always being deleted, causing the cache directory to grow to over 400 GB. To address this, an EXIT trap was added to HippUnfoldHCP.sh so that the subject-specific job cache is removed whenever the script exits, including following an error.
   Added a Snakemake unlock step to remove stale locks that may remain after incomplete or failed previous runs and prevent subsequent runs from proceeding.
Added the --latency-wait 60 and --rerun-incomplete flags to the HippUnfold command to improve robustness against common causes of failed or interrupted runs.
2. RunPostHippUnfoldHCP.sh
   Added a check to verify that HippUnfold completed successfully for each subject before initiating post-processing. Subjects without a successfully completed HippUnfold run are excluded from post-processing, and the user is notified.

3. PostHippUnfoldHCP.sh
   Changed the output directory name from native to Native.
   Completed the spec-file merge section. Previously, merged cortical + hippocampal spec files were created only for native (T1w + MNI), 512 (T1w + MNI), and 2k (T1w only). The missing merges were added for the remaining meshes and spaces.
Potentially fixed a bug in the transformation of hippocampal surfaces from T1w/physical space to MNI space. The wb_command -surface-apply-warpfield command supplied an MRI image (T1w_restore.nii.gz) as the argument to -fnirt, rather than the corresponding FNIRT transformation field. The original command contained: wb_command -surface-apply-warpfield "${AtlasFolder}/xfms/standard2acpc_dc.nii.gz" -fnirt "${AtlasFolder}/T1w_restore.nii.gz" The -fnirt argument was changed to: -fnirt "${AtlasFolder}/xfms/acpc_dc2standard.nii.gz" Deleted and recreated all HippUnfold directories for the 45 subjects in both YA_HCP_Final and YA_HCP_ReTest_Final using the current version of the script. As a result, spec files generated by older versions of the script, which included additional NIfTI files such as T1w or T2w images without the _restore suffix, are no longer present. PostHippUnfoldHCP.sh previously used two-space indentation. To conform to Tim's coding standard, the entire script was reformatted to use four-space indentation.

Questions:
1. Should QUEUE be defined in SetUpHCPPipeline.sh rather than directly in the run script?

2. In PostHippUnfoldHCP.sh, for the 2k, 8k, and 18k hippocampal meshes, I merged the hippocampal spec files with the 164k cortical MSMAll spec file. Is 164k the correct cortical spec file to use for all three hippocampal mesh resolutions?

3. Matt — you said that only the T1w and T2w NIfTI files should be included. I also see T1wDividedByT2w.nii.gz and T1wDividedByT2w_ribbon.nii.gz in the spec files. Should these also be removed?

Those can stay.

glasserm · 2026-09-19T20:26:58Z

I would pair 8k and 18k with 164k meshes, since they are of more similar sampling density.

Otherwise things looked good for my review of the test dataset.

glasserm · 2026-09-19T20:36:35Z

The PostHippUnfold diff is still full of non-changes. Can you clean it up so that I can see only the stuff that actually changed?

Update HippUnfold HCP processing

0b7b658

coalsont reviewed Aug 26, 2026

View reviewed changes

Comment thread HippUnfoldHCP/HippUnfoldHCP.sh Outdated

coalsont reviewed Aug 26, 2026

View reviewed changes

Comment thread HippUnfoldHCP/HippUnfoldHCP.sh Outdated

coalsont reviewed Aug 26, 2026

View reviewed changes

Comment thread HippUnfoldHCP/HippUnfoldHCP.sh Outdated

coalsont reviewed Aug 26, 2026

View reviewed changes

Comment thread HippUnfoldHCP/PostHippUnfoldHCP.sh Outdated

Address HippUnfold review comments

9f9f02b

coalsont reviewed Aug 28, 2026

View reviewed changes

Comment thread Examples/Scripts/RunPostHippUnfoldHCP.sh Outdated

coalsont reviewed Aug 28, 2026

View reviewed changes

Comment thread Examples/Scripts/RunHippUnfoldHCP.sh Outdated

coalsont reviewed Aug 28, 2026

View reviewed changes

Comment thread Examples/Scripts/RunHippUnfoldHCP.sh Outdated

coalsont reviewed Aug 28, 2026

View reviewed changes

Comment thread Examples/Scripts/RunPostHippUnfoldHCP.sh Outdated

glasserm reviewed Aug 31, 2026

View reviewed changes

Comment thread Examples/Scripts/RunHippUnfoldHCP.sh Outdated

glasserm reviewed Aug 31, 2026

View reviewed changes

Comment thread Examples/Scripts/RunPostHippUnfoldHCP.sh Outdated

glasserm reviewed Aug 31, 2026

View reviewed changes

Comment thread Examples/Scripts/RunPostHippUnfoldHCP.sh Outdated

glasserm reviewed Aug 31, 2026

View reviewed changes

Comment thread Examples/Scripts/RunPostHippUnfoldHCP.sh Outdated

glasserm requested changes Aug 31, 2026

View reviewed changes

coalsont reviewed Sep 1, 2026

View reviewed changes

Comment thread HippUnfoldHCP/HippUnfoldHCP.sh Outdated

Update HippUnfold processing

3b9ddb4

Fix HippUnfold run scripts

3413238

coalsont reviewed Sep 9, 2026

View reviewed changes

Comment thread Examples/Scripts/RunPostHippUnfoldHCP.sh Outdated

coalsont reviewed Sep 9, 2026

View reviewed changes

Update HippUnfold processing

02bf91c

Update HippUnfold processing

dcc4340

coalsont reviewed Sep 19, 2026

View reviewed changes

		#!/bin/bash

Conversation

orenpoliva commented Aug 25, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

glasserm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

orenpoliva commented Sep 8, 2026

Uh oh!

coalsont commented Sep 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glasserm Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coalsont Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

orenpoliva commented Sep 18, 2026

Uh oh!

coalsont commented Sep 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glasserm commented Sep 19, 2026

Uh oh!

glasserm commented Sep 19, 2026

Uh oh!

glasserm commented Sep 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glasserm commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

glasserm Sep 19, 2026 •

edited

Loading

coalsont Sep 9, 2026 •

edited

Loading

glasserm commented Sep 19, 2026 •

edited

Loading