Skip to content

[FIX] path generated by BABS is too long for freesurfer to function/run properly #138

Open
@yibeichan

Description

@yibeichan
Collaborator

as mentioned in this issue #137 i'm using fmriprep thru babs and encountered problem with midthickness.

Cmdline:
	mris_expand -pial /om2/scratch/Sun/yibei/budapest/output/job-32194722-sub-sid000030/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000030_wf/anat_preproc_wf/surface_recon_wf/gifti_surface_wf/midthickness/mapflow/_midthickness0/lh.pial -thickness -thickness_name /om2/scratch/Sun/yibei/budapest/output/job-32194722-sub-sid000030/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000030_wf/anat_preproc_wf/surface_recon_wf/gifti_surface_wf/midthickness/mapflow/_midthickness0/lh.thickness /om2/scratch/Sun/yibei/budapest/output/job-32194722-sub-sid000030/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000030_wf/anat_preproc_wf/surface_recon_wf/gifti_surface_wf/midthickness/mapflow/_midthickness0/lh.white 0.5 /om2/scratch/Sun/yibei/budapest/output/job-32194722-sub-sid000030/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000030_wf/anat_preproc_wf/surface_recon_wf/gifti_surface_wf/midthickness/mapflow/_midthickness0/lh.midthickness
Stdout:
	reading pial surface from /om2/scratch/Sun/yibei/budapest/output/job-32194722-sub-sid000030/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000030_wf/anat_preproc_wf/surface_recon_wf/gifti_surface_wf/midthickness/mapflow/_midthickness0/lh.pial
	using distance as a % of thickness
	using thickness file /om2/scratch/Sun/yibei/budapest/output/job-32194722-sub-sid000030/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000030_wf/anat_preproc_wf/surface_recon_wf/gifti_surface_wf/midthickness/mapflow/_midthickness0/lh.thickness
	expanding surface /om2/scratch/Sun/yibei/budapest/output/job-32194722-sub-sid000030/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000030_wf/anat_preproc_wf/surface_recon_wf/gifti_surface_wf/midthickness/mapflow/_midthickness0/lh.white by 50.0% of thickness and writing it to /om2/scratch/Sun/yibei/budapest/output/job-32194722-sub-sid000030/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000030_wf/anat_preproc_wf/surface_recon_wf/gifti_surface_wf/midthickness/mapflow/_midthickness0/lh.midthickness
	reading thickness...
Stderr:
	*** buffer overflow detected ***: terminated
	Aborted (core dumped)
Traceback:
	RuntimeError: subprocess exited with code 134.

I reported the details about this error on neurostars (I thought it was a fmriprep/freesurfer issue)
it turns out that the .git folder having a period in it is causing issues

@zhao-cy you have tested BABS with freesurfer before, right? did you encounter anything similar?

Activity

yibeichan

yibeichan commented on Aug 31, 2023

@yibeichan
CollaboratorAuthor

ah, is it because the path is too long for freesurfer? @mattcieslak

mattcieslak

mattcieslak commented on Aug 31, 2023

@mattcieslak
Collaborator

unfortunately it looks like it. This bug is awful

yibeichan

yibeichan commented on Aug 31, 2023

@yibeichan
CollaboratorAuthor

oh no... that doesn't sound good... let me talk to @djarecka tomorrow first and see whether we can come up with some plans to fix it. will keep you updated

changed the title [-]midthickness (freesurfer) failed during fmriprep due to `.git` in the path[/-] [+][FIX] path generated by BABS is too long for freesurfer to function/run properly[/+] on Sep 1, 2023
zhao-cy

zhao-cy commented on Sep 21, 2023

@zhao-cy
Collaborator

I'm closing the issue for now, as it seems the lengthy path is mainly generated by fMRIPrep, so it's probably out of the scope of BABS.

yibeichan

yibeichan commented on Sep 25, 2023

@yibeichan
CollaboratorAuthor

some updates for this one, see my post here, maybe BABS can do something to make this path shorter as it does in The Script?

zhao-cy

zhao-cy commented on Sep 27, 2023

@zhao-cy
Collaborator

Hi Yibei! @mattcieslak suggested this: since the issue is when running singularity run, maybe bind the main folder to singularity, so that it won't appear long within singularity? i.e., your original path: /om2/scratch/Sun/yibei/budapest/output/job-32194541-sub-sid000005/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000005_wf/anat_preproc_wf/surface_recon_wf/gifti_surface_wf/midthickness/mapflow/_midthickness0

And for the singularity run command in analysis/code/fmriprep*_zip.sh, add a line of -B /om2/scratch/Sun/yibei/budapest/output/job-32194541-sub-sid000005/ds:/data, i.e., bind the path as a folder called /data or so within the singularity. The Job ID can be retrieved by some env variable, and subject ID is also available in that script. Please make sure you datalad save the script before moving forward.

After changing this, you might or might not need to change other relative paths in the scripts - may be try out and see?

yibeichan

yibeichan commented on Sep 28, 2023

@yibeichan
CollaboratorAuthor

Hi Chenying! Yes, I agree, it should be something to do with binding.
See the code (original) below, it already bound ${PWD}, which is something like /om2/scratch/Sun/yibei/budapest/output/job-32194541-sub-sid000005/ds/

if I change that line to -B ${PWD}:/any_dirname \, I get fmriprep: error: Path does not exist: <inputs/data/BIDS>.

mkdir -p ${PWD}/.git/tmp/wkdir
singularity run --cleanenv \
	-B ${PWD} \
	-B /om2/user/$(whoami)/images/license.txt:/SGLR/FREESURFER_HOME/license.txt \
	containers/.datalad/environments/fmriprep-23-1-4/image \
	inputs/data/BIDS \
	outputs/fmriprep \
	participant \
	-w ${PWD}/.git/tmp/wkdir \
	--stop-on-first-crash \
	--nthreads 16 \
	--omp-nthreads 8 \
	--mem-mb 40000 \
	--fd-spike-threshold 0.9 \
	--dvars-spike-threshold 5 \
	--fs-license-file /SGLR/FREESURFER_HOME/license.txt \
	--skip-bids-validation \
	--output-layout legacy \
	--force-bbr \
	--notrack \
	--cifti-output 91k \
	-v -v \
	--participant-label "${subid}"
mattcieslak

mattcieslak commented on Sep 28, 2023

@mattcieslak
Collaborator

You'd need to change your command to

singularity run --cleanenv \
	-B ${PWD}/inputs/data/BIDS:/bids_data \
        -B ${PWD} \
        -B ${PWD}/.git/tmp/wkdir:/work \
	-B /om2/user/$(whoami)/images/license.txt:/SGLR/FREESURFER_HOME/license.txt \
	containers/.datalad/environments/fmriprep-23-1-4/image \
	/bids_data \
	outputs/fmriprep \
	participant \
	-w /work \
	--stop-on-first-crash \
	--nthreads 16 \
	--omp-nthreads 8 \
	--mem-mb 40000 \
	--fd-spike-threshold 0.9 \
	--dvars-spike-threshold 5 \
	--fs-license-file /SGLR/FREESURFER_HOME/license.txt \
	--skip-bids-validation \
	--output-layout legacy \
	--force-bbr \
	--notrack \
	--cifti-output 91k \
	-v -v \
	--participant-label "${subid}"
yibeichan

yibeichan commented on Sep 28, 2023

@yibeichan
CollaboratorAuthor

thanks! it works!

before changing, fmriprep node path is /om2/scratch/tmp/yibei/budapest_babs/job-32807164-sub-sid000009/ds/.git/tmp/wkdir/fmriprep_23_1_wf/single_subject_sid000009_wf/anat_preproc_wf/brain_extraction_wf/full_wm
after this change: /work/fmriprep_23_1_wf/single_subject_sid000010_wf/anat_preproc_wf/brain_extraction_wf/full_wm

I think this is the solution then!

should we make a PR for babs? I can work on it if @zhao-cy tells me which file I should look into.

mattcieslak

mattcieslak commented on Sep 28, 2023

@mattcieslak
Collaborator

I think it might be our only option to ensure the filenames are ok, but it does have some negative side effects.

For example, if someone looks through their error logs, they'll see an error regarding a file in /work/something and will not know where this file is on their file system. If we go this route we should definitely add some documentation to help explain the new file paths!

yibeichan

yibeichan commented on Sep 28, 2023

@yibeichan
CollaboratorAuthor

sounds good! (now thinking where do we want to put this documentation, do we want a separate FAQ page?

zhao-cy

zhao-cy commented on Sep 28, 2023

@zhao-cy
Collaborator

Hi Yibei! How about we create a new webpage, called What if something goes wrong? or so, in parallel with other docs (like overview, walkthrough), and include potential "failures" when using BABS? Let me know if you'd like to start that or I can also do it later.

yibeichan

yibeichan commented on Sep 28, 2023

@yibeichan
CollaboratorAuthor

sound good! I can do it, mostly likely next week unless I get bored tomorrow then I'll do it tomorrow (very low probability haha)

yibeichan

yibeichan commented on Sep 28, 2023

@yibeichan
CollaboratorAuthor

btw, this is where related to binding, right? and we need more than one -B

babs/babs/utils.py

Lines 431 to 432 in 6d6e5d9

# Get env var's value, to be used for binding `-B` in `singularity run`:
env_var_value = os.getenv(env_var_name)

zhao-cy

zhao-cy commented on Sep 28, 2023

@zhao-cy
Collaborator

Notes:

  • There will be multiple functions to be changed - need to spend some time to identify them;
  • also we need to consider how to handle the case of more than one input dataset (e.g., FreeSurfer ingressed one).
  • For docs: can make a webpage of FAQs or so to explain the newly defined paths. (sorry I had a bit misunderstanding earlier)

Current plan: enhance this before the next major release of BABS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationenhancementNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @mattcieslak@zhao-cy@yibeichan

        Issue actions

          [FIX] path generated by BABS is too long for freesurfer to function/run properly · Issue #138 · PennLINC/babs