Skip to content

+Move longitudinal FreeSurfer outputs to longitudinal session folders…#374

Open
mmilch01 wants to merge 26 commits intomasterfrom
longitudinal-file-management-patch
Open

+Move longitudinal FreeSurfer outputs to longitudinal session folders…#374
mmilch01 wants to merge 26 commits intomasterfrom
longitudinal-file-management-patch

Conversation

@mmilch01
Copy link
Copy Markdown
Contributor

@mmilch01 mmilch01 commented Dec 26, 2025

See bc88380. This needs review before merging.

+Move longitudinal FreeSurfer outputs to longitudinal session folders instead of symlinking
+remove target transform if it was created as symlink from a read-only source
+clean up large files in longitudinal diffusion output (NEEDS REVIEW)

… instead of symlinking

+remove target transform if it was created as symlink from a read-only source
+clean up large files in longitudinal diffusion output (NEEDS REVIEW)
@glasserm
Copy link
Copy Markdown
Contributor

I don't think we need copies of much of anything from the cross-sectional ${Subject} folders persisting in the longitudinal ${Subject} folders. I suspect these copies and symlinks were being done mainly to avoid needing to figure out exactly what files needed to be temporarily used. In the longitudinal folders, every single file (except I guess files that are time only and have no space), will need to either be in the template aligned T1w space or the new MNI space. I would be better able to assist by reviewing the output files rather than the code.

Comment thread DiffusionPreprocessing/DiffPreprocPipeline.sh
Copy link
Copy Markdown
Contributor

@glasserm glasserm left a comment

Choose a reason for hiding this comment

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

These look fine to me.

Comment thread PostFreeSurfer/PostFreeSurferPipelineLongPrep.sh Outdated
Comment thread PostFreeSurfer/PostFreeSurferPipelineLongPrep.sh Outdated
…, used by longitudinal FS

+remove 'fsaverage' symlink in template's T1s folder, created by FS
+added comments with example timepoint folder names in longitudinal Freesurfer and HCP structure.
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated
Comment thread PostFreeSurfer/PostFreeSurferPipelineLongPrep.sh
@mharms
Copy link
Copy Markdown
Contributor

mharms commented Jan 27, 2026

Why does the moving of the per-session FS longitudinal output occur in the PostFreeSurferPipelineLongPrep.sh script, rather than as part of LongitudinalFreeSurferPipeline.sh? It seems to me that belongs in the latter (with the PostFreeSurferPipelineLongPrep.sh script then just written to look in that location if it needs any of those files)

# tp_folder_fslong stores folder where longitudinal FreeSurfer stores timepoint output natively.
# Example of how it would look like, given Template=123456_V1_V2, Subject=123456, Timepoint=123456_V1:
# $tp_folder_fslong=$StudyFolder/123456.long.123456_V1_V2/T1w/123456_V1.long.123456_V1_V2
tp_folder_fslong="${LongDIR}/${Timepoint_cross}.long.${Template}"
if [ ! -d "$tp_folder_fslong" ]; then
log_Err_Abort "Folder $tp_folder_fslong does not exist, was longitudinal FreeSurfer run?"
# tp_folder_hcp contains target timepoint folder in HCP pipelines.
# $tp_folder_hcp=$StudyFolder/123456_V1.long.123456_V1_V2/T1w/123456_V1.long.123456_V1_V2
tp_folder_hcp="${TargetDIR}/${Timepoint_cross}.long.${Template}"
if [ -d "$tp_folder_hcp" ]; then
rm -rf "$tp_folder_hcp"
fi
mv "$tp_folder_fslong" "$tp_folder_hcp"
fi

@coalsont
Copy link
Copy Markdown
Member

Just speculating, but if we have already run LongFS and don't want to rerun it, then PostFSLong will have to deal with whatever conventions it used regardless. But, it might still be a good idea to also edit LongFS to put things in their final location when possible (though this means making sure PostFSLong can deal with both situations).

@mharms
Copy link
Copy Markdown
Contributor

mharms commented Jan 27, 2026

None of this has been run yet "in production", so I feel no need to maintain any backwards compatibility here. Unless we are using an FS tool that requires an FS-style SUBJECTS_DIR organization, I don't see any reason why we shouldn't put the moving of the per-session LongFS outputs into the LongFS script itself, and then just modify (and simplify) PostFSLong to find the data in that location.

@mmilch01
Copy link
Copy Markdown
Contributor Author

mmilch01 commented Feb 2, 2026

This 'why' has mainly historical reasons, so I agree it would be cleaner to move longitudinal session dirs in the end of longitudinal FS. Since the number of changes in this PR already calls for a full retest of longitudinal stream, it's probably fine to change now rather than later.

UPD: addressed here: d935cd3

Mikhail Milchenko and others added 4 commits February 2, 2026 14:07
+fix situation where non-existent "$MNIFolder/Results/$extractNameOut" folder breaks longitudinal processing.
Comment thread Examples/Scripts/DiffusionPreprocessingBatch-long.sh Outdated
Comment thread FreeSurfer/custom/conf2hires Outdated
Comment thread PostFreeSurfer/scripts/GenerateStructuralScenes.sh Outdated
Comment thread TaskfMRIAnalysis/scripts/makeSubjectTaskSummary.sh Outdated
Comment thread TransmitBias/scripts/PseudoTransmit_IndividualAlignRawAndInitialMaps.sh Outdated
Comment thread TransmitBias/scripts/PseudoTransmit_IndividualAlignRawAndInitialMaps.sh Outdated
Comment thread fMRIVolume/GenericfMRIVolumeProcessingPipeline.sh Outdated
Comment thread fMRIVolume/GenericfMRIVolumeProcessingPipeline.sh Outdated
Comment thread TaskfMRIAnalysis/scripts/makeSubjectTaskSummary.sh
Comment thread Examples/Scripts/DiffusionPreprocessingBatch-long.sh
@coalsont
Copy link
Copy Markdown
Member

It looks like there is a conflict from adding longitudinal -conf2hires to master since this branch was created. Please figure out a way to make github happy about it (manual rebase with conflict resolution edit and force push, or see what the github "resolve conflicts" button does).

@mmilch01
Copy link
Copy Markdown
Contributor Author

mmilch01 commented Apr 1, 2026

It looks like there is a conflict from adding longitudinal -conf2hires to master since this branch was created. Please figure out a way to make github happy about it (manual rebase with conflict resolution edit and force push, or see what the github "resolve conflicts" button does).

Resolved in 3fa8604

@coalsont
Copy link
Copy Markdown
Member

coalsont commented Apr 2, 2026

There appears to be potential for conflict with the NHP fMRIVolume and Diffusion PRs (some of the same files edited). Maybe this PR should wait?

Comment thread fMRIVolume/GenericfMRIVolumeProcessingPipeline.sh Outdated
Comment thread fMRIVolume/GenericfMRIVolumeProcessingPipeline.sh Outdated
Comment thread fMRIVolume/GenericfMRIVolumeProcessingPipeline.sh
Comment thread fMRIVolume/GenericfMRIVolumeProcessingPipeline.sh Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants