initial draft of neurodocker support

bids-apps / freesurfer

BIDS app wrapping recon-all from FreeSurfer

Apache License 2.0

40 stars 35 forks source link

initial draft of neurodocker support #55

Closed PeerHerholz closed 4 years ago

PeerHerholz commented 4 years ago

this PR introduces the first draft of a potential neurodocker support
it changes/adds the following files: generate_freesurfer_images.sh, Dockerfile, Singularity
generate_freesurfer_images.sh is a bash script that utilizes neurodocker to create the Dockerfile and Singularity recipe
Dockerfile is the Dockerfile created through generate_freesurfer_images.sh
Singularity is the Singularity recipe created through generate_freesurfer_images.sh

Shotgunosine commented 4 years ago

Tests are failing because Neurodocker's Freesurfer is looking for a license at /opt/freesurfer/license.txt. We could copy the license file to that location in the circlci tests.

Shotgunosine commented 4 years ago

Also, it does build for me, I was able to get the first test running with this command: docker run -ti --rm --read-only -v ~/license/license.txt:/opt/freesurfer/license.txt --tmpfs /tmp --tmpfs /var/tmp -v ${HOME}/data/ds114_test1:/bids_dataset -v ${HOME}/outputs1:/outputs bids/${CIRCLE_PROJECT_REPONAME} /bids_dataset /outputs participant --participant_label 01 --license_file=/opt/freesurfer/license.txt --stages autorecon1 && cat ${HOME}/outputs1/sub-01/scripts/recon-all.done

PeerHerholz commented 4 years ago

Tests are failing because Neurodocker's Freesurfer is looking for a license at /opt/freesurfer/license.txt. We could copy the license file to that location in the circlci tests.

Yes, sounds about right. Should I do that or do you want to check it? We then would also have to update the readme, indicating the specific path the license should be mapped to.

Shotgunosine commented 4 years ago

I'll give it a shot, may be able to just use 6.0.1 instead of 6.0.0-min.

PeerHerholz commented 4 years ago

I'll give it a shot, may be able to just use 6.0.1 instead of 6.0.0-min.

AFAIK neurodocker currently "only" supports version 6.0.0-min. Might be worth opening an issue for that in the corresponding repo.

Shotgunosine commented 4 years ago

Nah, you can specify 6.0.1, you just don't get a minified version, which is fine.

PeerHerholz commented 4 years ago

True that, just checked it. Their readme needs a respective update. Will open an issue there.

PeerHerholz commented 4 years ago

Looks about right, reopening for review. Thx @Shotgunosine!

PeerHerholz commented 4 years ago

Tests are passing and I addressed your comments. Hence, I think we can go ahead and merge, no? @Shotgunosine

Shotgunosine commented 4 years ago

Yep, I think so.

On Thu, Jun 4, 2020 at 8:59 PM Peer Herholz notifications@github.com wrote:

Tests are passing and I addressed your comments. Hence, I think we can go ahead and merge, no? @Shotgunosine https://github.com/Shotgunosine

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/BIDS-Apps/freesurfer/pull/55#issuecomment-639196550, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABILLGEMQIHX6BMWW4TNX5TRVA7NRANCNFSM4NQDW2AQ .