Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider splitting post-processing from heudiconv #58

Open
tsalo opened this issue Aug 1, 2020 · 4 comments
Open

Consider splitting post-processing from heudiconv #58

tsalo opened this issue Aug 1, 2020 · 4 comments

Comments

@tsalo
Copy link
Member

tsalo commented Aug 1, 2020

Now that we have a workaround for operating on datasets in /home/data, we don't actually need a lot of the extra work we originally had in cis-bidsify. We've already simplified the workflow, but this has made it clear that we probably don't need to include heudiconv inside our build. Specifically, I'm proposing that we start using a heudiconv Singularity image, followed by a streamlined version of cis-bidsify, which would only handle our post-processing steps (i.e., defacing, metadata completion, metadata cleanup, and validation).

@tsalo
Copy link
Member Author

tsalo commented Aug 1, 2020

One sticking point for me has been that heudiconv's temporary directories have ended up in weird places, since it uses TMPDIR to decide where they should go. I got around this by setting TMPDIR in the cis-bidsify workflow. However, I recently became aware of SINGULARITYENV_*, which can be set in conjunction with --cleanenv.

Here's an example use:

$ export SINGULARITYENV_FOO=bar

$ singularity exec --cleanenv alpine_latest.sif env
SHLVL=1
LD_LIBRARY_PATH=/.singularity.d/libs
HOME=/home/vagrant
PS1=Singularity>
TERM=xterm-256color
PATH=/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/usr/local/sbin
FOO=bar
LANG=C
SINGULARITY_APPNAME=
SINGULARITY_CONTAINER=/home/vagrant/alpine_latest.sif
PWD=/home/vagrant
SINGULARITY_NAME=alpine_latest.sif

Note that FOO is set within the container based on SINGULARITYENV_FOO from outside the container, even though we used cleanenv.

@tsalo
Copy link
Member Author

tsalo commented Aug 2, 2020

Example usage would be

tmpdir=/home/data/nbc/Laird_DIVA/work-dir/${sub}-${ses}/
export SINGULARITYENV_TMPDIR=$tmpdir
singularity exec --cleanenv nipy_heudiconv_latest.sif -d X -s $sub -ss $ses -f X --datalad -b -o X

We could also maybe address #54 with export SINGULARITYENV_PWD too.

@akimbler
Copy link
Member

akimbler commented Sep 6, 2020

@tsalo, I've kind of flipped my opinion on this. If we remove the heudiconv processing the package is kind of barebones. I vote that we keep the workflow in it's current state, heudiconv included.

@tsalo
Copy link
Member Author

tsalo commented Sep 6, 2020

If our own additions to heudiconv are small because heudiconv does almost everything we need, then that's a good thing, in my opinion. A larger tool like heudiconv should always be our go-to for adding features, and we should only add things to our own workflow when the heudiconv devs don't want them or the features we need are specific to the CIS or HPC.

Since we figured out that Singularity workaround, there aren't many things in the workflow that are included because they're CIS/HPC-specific. Instead, we can really just focus on the stuff that is outside of heudiconv's scope.

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

No branches or pull requests

2 participants