Open
Conversation
glasserm
requested changes
Dec 15, 2025
Contributor
glasserm
left a comment
glasserm
left a comment
There was a problem hiding this comment.
My overall comment is that I think we should properly merge the NHP changes in to the fMRIVolume pipeline rather than bifurcate the pipelines. There is a lot of reused code between fMRIVolume and the BBR code and these are not really a completely separate process like they were for FreeSurferNHP. On initial review the changes to one step resampling and motion correction looked okay.
coalsont
reviewed
Dec 16, 2025
Contributor
Author
|
Thank you for the comment — that makes sense. To confirm, should After discussing with @takuya-hayashi, we felt that this script includes quite a lot of NHP-specific logic. |
Contributor
Yes please. |
coalsont
reviewed
Feb 2, 2026
coalsont
reviewed
Feb 3, 2026
coalsont
reviewed
Feb 3, 2026
coalsont
reviewed
Feb 3, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
glasserm
reviewed
Feb 6, 2026
coalsont
reviewed
Feb 7, 2026
coalsont
reviewed
Feb 7, 2026
coalsont
reviewed
Feb 7, 2026
coalsont
reviewed
Feb 7, 2026
coalsont
reviewed
Feb 7, 2026
coalsont
reviewed
Feb 7, 2026
coalsont
reviewed
Feb 7, 2026
…urferPipelineBatchNHP.sh
- Update jacobian comment: "handled below, and is optional" (r2772048473)
- Fix typo "filed" -> "field" (r2772049269)
- Remove empty else clause (r2772057701)
- Invert Human check to != Human pattern in OneStepResampling.sh (r2772060840)
- Add --species argument to DistortionCorrection script (r2776907266)
- Derive betspecieslabel from SPECIES when not set (r2776912100)
- Pass --species=${SPECIES} from pipeline to DistortionCorrection call
- Fix pre-existing syntax: [ && ] -> [[ && ]], missing space before ],
missing fi for DistortionCorrection != SPIN_ECHO block
- Fix ${10}/${11} positional parameter expansion in MotionCorrection.sh
- Fix IsLongitudinal condition that always evaluated False
- Add --species option to OneStepResampling.sh
- Delete unused GenericfMRIVolumeMotionCorrectXRunsNHP.sh
- Rename --bbr/BBR to --bbr-contrast/BBRContrast for clarity - Fix jacobian comment in human code path, remove in NONE path - Use (( ! IsLongitudinal )) syntax in DistortionCorrection script - Restructure OneStepResampling.sh: full else for human/nonhuman - Revert multi-echo code changes: restore else clause, remove duplicate
Complete the reversion of echo array changes that was flagged as incomplete in PR Washington-University#372 review comments r2853580713 and r2853593232.
This reverts commit 8eb67af.
… @ parsing - Add StructRes, isFLAIR, isT1wDivFLAIR variables - Pass --structres to SetUpSPECIES.sh, pass args to SetUpFSNHP.sh - Remove unnecessary @-delimiter stripping from session variables
TakJim
force-pushed
the
NHPUnifiedFMRIVolume
branch
from
3755989 to
926a14c
Compare
Contributor
Author
|
Rebased onto current master ( |
- Move Jacobian/registration/warpfield/SEBASED code (was shared) into the else # for Human block in TOPUP case - Move UseJacobian+BiasField block into else # Human block in BBR section - Delete old NHP stub log messages replaced by new NHP processing
mmilch01
reviewed
Apr 13, 2026
| # These are set by SetUpSPECIES.sh (sourced after EnvironmentScript). | ||
| # Override here only if you need non-default values for your dataset. | ||
| # Example values for Macaque: | ||
| # FinalFMRIResolution="1.25" |
Contributor
There was a problem hiding this comment.
Suggested change
| # FinalFMRIResolution="1.25" | |
| # FinalfMRIResolution="1.25" |
change to match SetUpSpecies.sh
mmilch01
reviewed
Apr 13, 2026
| #CorticalScaleFactor="1" # Cortical scale factor | ||
| # | ||
| #### fMRIVolume-relevant variables | ||
| #FinalFMRIResolution="2" # Target final resolution of fMRI data in mm |
Contributor
There was a problem hiding this comment.
Suggested change
| #FinalFMRIResolution="2" # Target final resolution of fMRI data in mm | |
| #FinalfMRIResolution="2" # Target final resolution of fMRI data in mm |
change to match SetUpSpecies.sh
mmilch01
reviewed
Apr 13, 2026
| --echospacing="$DwellTime" \ | ||
| --echodiff="$DeltaTE" \ | ||
| --unwarpdir="$UnwarpDir" \ | ||
| --fmrires="$FinalFMRIResolution" \ |
Contributor
There was a problem hiding this comment.
Suggested change
| --fmrires="$FinalFMRIResolution" \ | |
| --fmrires="$FinalfMRIResolution" \ |
change to match SetUpSpecies.sh
mmilch01
reviewed
Apr 13, 2026
| --echospacing=$DwellTime \ | ||
| --echodiff=$DeltaTE \ | ||
| --unwarpdir=$UnwarpDir \ | ||
| --fmrires=$FinalFMRIResolution \ |
Contributor
There was a problem hiding this comment.
Suggested change
| --fmrires=$FinalFMRIResolution \ | |
| --fmrires=$FinalfMRIResolution \ |
change to match SetUpSpecies.sh
mmilch01
reviewed
Apr 13, 2026
| # Scaling brain size to adapt to size dependency of mcflirt - TH 2024 | ||
| if [[ ! -z "$BrainScaleFactor" && ! $(echo "$BrainScaleFactor == 1" | bc) = 1 ]] ; then | ||
| log_Msg "Scaling brain with a factor = $BrainScaleFactor" | ||
| ${HCPPIPEDIR_Global}/ScaleVolume.sh ${Scout}.nii.gz ${BrainScaleFactor} ${WorkingDirectory}/scout_scale.nii.gz ${WorkingDirectory}/scale.world.mat |
Contributor
There was a problem hiding this comment.
Suggested change
| ${HCPPIPEDIR_Global}/ScaleVolume.sh ${Scout}.nii.gz ${BrainScaleFactor} ${WorkingDirectory}/scout_scale.nii.gz ${WorkingDirectory}/scale.world.mat | |
| ${HCPPIPEDIR_Global}/ScaleVolumeNHP.sh ${Scout}.nii.gz ${BrainScaleFactor} ${WorkingDirectory}/scout_scale.nii.gz ${WorkingDirectory}/scale.world.mat |
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
5 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 applies fMRIVolume to NHP data.
Some additional modifications are still required. The parts that I have not been able to address yet are listed below.
TODO