NHP DiffusionPreprocessing#388
Open
TakJim wants to merge 28 commits into
Open
Conversation
Add NHP-adapted versions of the four DiffusionPreprocessing pipeline scripts: - DiffPreprocPipelineNHP.sh - DiffPreprocPipeline_EddyNHP.sh - DiffPreprocPipeline_PostEddyNHP.sh - DiffPreprocPipeline_PreEddyNHP.sh
- Replace --specieslabel (numeric) with --species (string, default "Human") matching PreFreeSurfer PR Washington-University#350 pattern - Add SPECIES to SpeciesLabel case mapping for NHP sub-scripts - Branch on SPECIES=="Human" vs !="Human" for PreEddy, Eddy, PostEddy: Human path calls original .sh with all HCP params (100% backward compat) NHP path calls *NHP.sh with NHP-specific params - validate_scripts(): retain all Human checks incl run_eddy.sh, add NHP script checks conditionally - Add --usephasezero pass-through to PreEddyNHP.sh - glibc workaround scoped to NHP path only
coalsont
reviewed
Mar 21, 2026
coalsont
reviewed
Mar 21, 2026
coalsont
reviewed
Mar 21, 2026
…ddy.sh - Add NHP options: --species, --specieslabel, --truepatientposition, --scannerpatientposition, --usephasezero - Add SPECIES branching for input data reorient (CorrectVolumeOrientation + Rotate_bvecs.sh for NHP, simple imcp for Human) - Add SPECIES branching for basic_preproc (NHP uses basic_preproc.sh, Human uses norm_intensity + best_b0/sequence) - Add SPECIES branching for topup (NHP: optional T2w phase-zero insertion + run_topupNHP.sh, Human: run_topup.sh) - Update main DiffPreprocPipeline.sh to call unified PreEddy for both Human and NHP, removing the PreEddyNHP.sh dispatch
- Add --species option to select Human or NHP eddy path - Human: run_eddy.sh with full GPU/CUDA support (unchanged) - NHP: run_eddyNHP.sh with -g flag and JacobianResampling marker - Update main DiffPreprocPipeline.sh to call unified Eddy for both Human and NHP, removing the EddyNHP.sh dispatch
…tEddy.sh - Add NHP options: --species, --specieslabel, --wmprojabs - SPECIES branching for eddy_postproc (Human: eddy_postproc.sh, NHP: eddy_postprocNHP.sh with SpeciesLabel) - SPECIES branching for DiffusionToStructural (Human: DiffusionToStructural.sh, NHP: DiffusionToStructuralNHP.sh with --fsbbrdiff and --wmprojabs) - SPECIES branching for eddy log copy and cnr_maps handling - Update main DiffPreprocPipeline.sh to call unified PostEddy for both Human and NHP, removing the PostEddyNHP.sh dispatch - All four Diffusion sub-scripts are now unified
The NHP-specific logic has been fully merged into the existing HCP DiffPreprocPipeline.sh / _PreEddy.sh / _Eddy.sh / _PostEddy.sh via SPECIES branching, so the separate NHP-suffixed entry points (DiffPreprocPipelineNHP.sh and its PreEddy/Eddy/PostEddyNHP counterparts) are no longer needed.
Per review feedback (coalsont), the glibc-2.14 path should not be in LD_LIBRARY_PATH by default, and if a site needs this workaround it belongs in their edited SetUpHCPPipeline.sh / Examples batch, not in a pipeline script.
Contributor
|
Please add the example batch file NHP. |
Contributor
Author
|
Added Modeled on
Species-specific default values (e.g., |
coalsont
reviewed
Apr 22, 2026
coalsont
reviewed
Apr 22, 2026
glasserm
reviewed
Apr 22, 2026
glasserm
reviewed
Apr 22, 2026
glasserm
reviewed
Apr 22, 2026
glasserm
reviewed
Apr 22, 2026
- Set EnvironmentScript at top with other batch settings, drop fallback default (coalsont review on PR Washington-University#388) - Move manual fallback block out of else-branch so it can be uncommented regardless of hcppipe_conf.txt presence (glasserm suggestion on PR Washington-University#388)
Adds --species option (default 'Human'). When non-Human: - append --data_is_shelled (NHP data often not perfectly shelled) - when --resamp=lsr, also append --fep (required for LSR on NHP) Towards integrating run_eddyNHP.sh into run_eddy.sh per glasserm review on PR Washington-University#388 (DiffPreprocPipeline_Eddy.sh:246).
Removes the Human/NHP if/else that called run_eddyNHP.sh separately. Now both species go through run_eddy.sh, gated by --species=$SPECIES. The JacobianResampling marker (used by PostEddy to detect LSR vs JAC) is kept behind an explicit SPECIES != Human guard since it is NHP-only behavior. Towards integrating run_eddyNHP.sh into run_eddy.sh per glasserm review on PR Washington-University#388 (DiffPreprocPipeline_Eddy.sh:246).
Adds 5th positional arg BetSpeciesLabel (default 0 = Human). When >=1, uses bet4animal with a species-specific bet fraction (0.3/0.4/0.5/0.6 for chimp/macaque/marmoset/other) and -z $BetSpeciesLabel, instead of standard 'bet -f 0.1'. Towards integrating eddy_postprocNHP.sh into eddy_postproc.sh per glasserm review on PR Washington-University#388 (DiffPreprocPipeline_PostEddy.sh:192).
Two NHP-only behaviors gated by BetSpeciesLabel != 0: - LSR resampling path: when JacobianResampling marker is absent, eddy has already combined Pos/Neg with LSR; skip eddy_quad QC and copy bvals/ bvecs/data through directly. - CNR map NaN rescue: spline-interpolated CNR maps can carry NaN on low-SNR NHP data; fall back to sinc interpolation if 'fslstats -m' reports NaN. Towards integrating eddy_postprocNHP.sh into eddy_postproc.sh per glasserm review on PR Washington-University#388 (DiffPreprocPipeline_PostEddy.sh:192).
Removes the Human/NHP if/else that called eddy_postprocNHP.sh separately. Now both species go through eddy_postproc.sh, with SpeciesLabel passed as the 5th positional arg (0 = Human, >=1 = NHP). Longitudinal Human skip path is preserved. Also drops the validate_scripts checks for the now-unified NHP variants (eddy_postprocNHP.sh and DiffusionToStructuralNHP.sh — the latter is removed here for symmetry; the call-site dispatch for DiffusionToStructural is updated in a follow-up commit). Towards integrating eddy_postprocNHP.sh per glasserm review on PR Washington-University#388 (DiffPreprocPipeline_PostEddy.sh:192).
…brdiff Adds three NHP-tunable getopt1 options (defaults selected so the Human path is unchanged): - --species (default 'Human') - --wmprojabs (default 2 mm; species-specific WM projection absolute distance for FreeSurfer bbregister) - --fsbbrdiff (default TRUE; set NONE to skip FS bbregister on low- quality NHP diffusion data) Behavioral changes that consume these options follow in subsequent commits. Towards integrating DiffusionToStructuralNHP.sh into DiffusionToStructural.sh per glasserm review on PR Washington-University#388 (DiffPreprocPipeline_PostEddy.sh:195).
For NHP (SPECIES != Human), run dtifit and use the predicted b=0 image (dti_S0) as the registration moving image instead of raw nodif. This gives more robust bbregister initialization on low-SNR diffusion data (TH 2016). Human path is unchanged (regimg=nodif). Towards integrating DiffusionToStructuralNHP.sh into the human version.
Two NHP-only behaviors gated by SPECIES != 'Human': - bbregister extra args: switch from --bold to --dti contact mode and, when WMProjAbs < 2, append '--wm-proj-abs $WMProjAbs --brute1max 2 --brute1delta 2' (species-tuned white-matter projection depth). - FSBBRDIFF=NONE fallback: skip FS bbregister + tkregister2 entirely and use the FLIRT BBR result (regimg2T1w_initII.mat) as diff2str.mat. Recommended for low-quality data or fieldmap-corrected data (TH). Human path is unchanged (uses --bold and the existing bbregister call). Towards integrating DiffusionToStructuralNHP.sh into the human version.
Removes the Human/NHP if/else that called DiffusionToStructuralNHP.sh separately. Both species now go through DiffusionToStructural.sh, with --species/--wmprojabs/--fsbbrdiff passed unconditionally (the NHP-only flags are no-ops on the Human path). Also refreshes the validate_scripts comment in DiffPreprocPipeline.sh to reflect that no separate *NHP.sh variants exist anymore. Towards integrating DiffusionToStructuralNHP.sh per glasserm review on PR Washington-University#388 (DiffPreprocPipeline_PostEddy.sh:195).
Per Matt issuecomment-4297611428 and Tim 4/23 NHP Pipelines MTG discussion: the FSBBRDIFF and DiffWMProjAbs settings were inadvertently dropped during a conflict resolution on PR Washington-University#364. They belong in this diffusion PR, not in the FreeSurfer PR. Restored 5 species sections matching the pre-conflict state at commit 9ae9e97: Human FSBBRDIFF=TRUE DiffWMProjAbs="2" Chimp FSBBRDIFF=TRUE DiffWMProjAbs="1" Macaque FSBBRDIFF=TRUE DiffWMProjAbs="0.7" Marmoset FSBBRDIFF=TRUE DiffWMProjAbs="0.5" NightMonkey FSBBRDIFF=TRUE DiffWMProjAbs="0.3" Verified the restored block content matches the reference 9ae9e97 byte-for-byte for all 5 species.
DiffPreprocPipeline_PreEddy.sh L500 calls run_topupNHP.sh on the NHP path, but the script was missing from this branch after the unification work. Copy it verbatim from the BCIL NHP staging tree (scripts/run_topupNHP.sh, 105 lines, 100755). This restores NHP runtime: bet4animal-based hifi b0 brain extraction with species-tuned BetSpeciesLabel + betfraction that the unified run_topup.sh (Human-only bet -f 0.2) doesn't cover. No Human-side change.
…ffusionToStructural Per Matt 4/23 NHP Pipelines MTG TODO E: bring the NHP-aware versions of the three dependency scripts into the PR Washington-University#388 branch by merging the integration-draft branch: 4ca1c38 run_eddy.sh: add --species and NHP-specific eddy flags 5488e58 DiffPreprocPipeline_Eddy.sh: unify SPECIES dispatch via run_eddy.sh 9dbd649 eddy_postproc.sh: add BetSpeciesLabel for NHP brain extraction 412d707 eddy_postproc.sh: add NHP LSR-resampling branch and CNR NaN rescue c6d6297 DiffPreprocPipeline_PostEddy.sh: unify eddy_postproc dispatch 07461f5 DiffusionToStructural.sh: add NHP options --species/--wmprojabs/--fsbbrdiff 4772528 DiffusionToStructural.sh: use dti_S0 as registration target for NHP e97495e DiffusionToStructural.sh: add NHP FS BBR options and fallback path ae7b97d DiffPreprocPipeline_PostEddy.sh: unify DiffusionToStructural dispatch 9cffd8d DiffusionPreprocessing: add run_topupNHP.sh The NHP path now references unified scripts gated by --species, with run_topupNHP.sh restored verbatim to satisfy the existing PreEddy.sh dispatch. No conflicts: this branch's 6ac2c21 only touched Examples/Scripts/SetUpSPECIES.sh.
Contributor
Author
Contributor
|
@coalsont why are there merge conflicts? |
glasserm
reviewed
Apr 30, 2026
Member
Because I added a few lines to the end of SetUpSPECIES in |
…mmented-out config-dispatch block - Remove commented-out SPECIES config-dispatch block (deprecated config selection now lives upstream in TopupConfig) - Drop deprecated_topup_config_file branches (variable was never defined; those code paths would have failed at runtime) - Drop partopupdir indirection (was immediately overwritten to $FSLDIR/bin); call topup directly via $FSLDIR/bin - No behavior change for paths that were actually exercised
…=0 default) - Add optional 3rd arg SpeciesLabel (defaults to 0 = Human, preserving the existing 2-arg signature for Human callers) - SpeciesLabel=0 path is byte-identical to the previous Human pathway - SpeciesLabel!=0 path is the cleaned-up NHP pathway from run_topupNHP.sh: parallel topup, T2w-phase-zero variant when Pos_Neg_NoZero_b0 is present, per-species BET fraction, bet4animal - Caller update and run_topupNHP.sh removal follow in the next commit
…opup.sh - Update NHP branch call site to use run_topup.sh with SpeciesLabel arg - Remove run_topupNHP.sh existence check (script no longer exists) - Delete DiffusionPreprocessing/scripts/run_topupNHP.sh
…rfer convention)
Match the PreFreeSurfer/ACPCAlignment.sh and BrainExtraction_FNIRTbased.sh
pattern: receive SPECIES as an argument and convert internally to the
numeric SpeciesLabel used by bet4animal -z, instead of having the caller
pre-compute and pass SpeciesLabel.
- run_topup.sh: 3rd arg is now SPECIES (defaults to Human). Internal
case-statement derives SpeciesLabel using the same mapping as
PreFreeSurfer scripts (Human=0, Chimp=1, Macaque=2, Marmoset=3,
NightMonkey=4).
- DiffPreprocPipeline_PreEddy.sh L497: pass ${SPECIES} instead of
${SpeciesLabel}.
- The 2-arg invocation (Human pathway) on L477 continues to work since
SPECIES defaults to Human inside run_topup.sh.
glasserm
requested changes
May 22, 2026
coalsont
reviewed
May 22, 2026
Add the combined NHP basic_preproc script as basic_preprocNHP.sh (naming consistent with eddy_postprocNHP/run_eddyNHP/run_topupNHP). Update DiffPreprocPipeline_PreEddy.sh NHP branch to call and validate basic_preprocNHP.sh instead of basic_preproc.sh.
Drop the Human/non-Human branch in DiffPreprocPipeline_PreEddy.sh so all
species share the split basic_preproc_{norm_intensity,best_b0,sequence}.sh
scripts. NHP-specific behaviour is handled via $SPECIES inside run_topup.sh
(now always passed) and the T2w phase-zero block, which is now explicitly
gated to non-Human. Delete the obsolete combined basic_preprocNHP.sh.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds NHP-adapted versions of the DiffusionPreprocessing pipeline scripts.
These scripts are the current working versions.
Files Added