-
Notifications
You must be signed in to change notification settings - Fork 532
ENH: use neurodocker 0.4.1 + apt install afni #2707
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
Conversation
This seems to be broken by #2691... |
@kaczmarj Is there a trivial way to trigger a new docker build? I want to make sure that things aren't only passing on master because we're reusing the old images. |
Built from scratch on |
Yeah, that fails. Looks like you're changing the Matlab and/or SPM installs, and our doc parsing is failing for these versions. How about we push this off to 1.1.4? Or possibly restore the old Matlab/SPM. |
@effigies - i think the issue is with the matlab compiler runtime installation. i'm fine pushing this to the next release, but i'll fix it soon. |
Codecov Report
@@ Coverage Diff @@
## master #2707 +/- ##
==========================================
+ Coverage 67.44% 67.87% +0.42%
==========================================
Files 340 340
Lines 43170 44002 +832
Branches 5352 5430 +78
==========================================
+ Hits 29116 29865 +749
- Misses 13356 13433 +77
- Partials 698 704 +6
Continue to review full report at Codecov.
|
Want to merge master? Circle seems to have done okay on Oscar's coverage update. |
The following command is failing.
One can run the following to reproduce
|
This is the specific error:
|
Neurodebian's FSL doesn't look to have been updated (correct @yarikoptic?). I wonder if there's a library bug that the new builds are hitting. |
Looks like the build is fixed on master. Unclear why. |
Ah, I see. Master is able to reuse the old base images, because of the compare_dockerfiles check. PRs from most users won't be able to do so. So fresh builds are still failing, we're just not doing them on master. |
I reproduced the issue. Adding
Any other thoughts? |
@effigies - i will see if using a neurodebian snapshot from a few months ago will solve the issue http://snapshot.debian.org/ |
@kaczmarj Any luck here? Is there something I can do to help this along? |
@kaczmarj Any chance we can figure this one out this week? Tests are still failing for all new PRs, and it would be nice to have this fixed before the next release. |
@effigies - i'm working on this now |
Thanks. Sorry to pester. |
Did some of my own tests on #2763. Even installing directly from FSL, flameo crashes for both 5.0.9 and 5.0.11. Perhaps it's a libc issue? IDK. Is there any chance of just fixing the broken aspects of the current Dockerfile? Or is this a minimal update already? |
@effigies - this is a minimal update already. i tried to update the original (neurodocker 0.3.2) dockerfile to use the non-neurodebian fsl to no avail. |
# neurodocker version 0.4.1-22-g7c44e01 | ||
NEURODOCKER_IMAGE="kaczmarj/neurodocker:master@sha256:858632a7533cac100f70932749b4cfc77fc40f667f41fca208f406215cff8a27" | ||
# neurodebian:stretch-non-free pulled on September 19, 2018 | ||
BASE_IMAGE="neurodebian:stretch-non-free@sha256:7cd978427d7ad215834fee221d0536ed7825b3cddebc481eba2d792dfc2f7332" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kaczmarj What if we revert the BASE_IMAGE
? If that helps, I think that would be decent evidence that it's a library version issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@effigies - i don't think that will help. the apt-get update
calls will update to the latest neurodebian stretch sources. i tried using neurodebian snapshots from september 17, 2017 and from january 1, 2017, but both ran into the same flameo
error. i'm going to try to use the nipype base image on dockerhub with the current nipype source code now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@effigies - i used nipype's base image on dockerhub (i.e., nipype/nipype:base
) and still got the flameo
error. is it possible that nipype's code is causing this? could one of the inputs to flameo
be corrupted / malformed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it works on master
, still, with the same inputs. There's something new in the build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kaczmarj Is there any chance of using a Debian snapshot from earlier, as well? If it's a libc update or similar that's causing the issue, that wouldn't be resolved by using a neurodebian snapshot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true, i'll try that now. i'm experimenting with updating packages in the current nipype py36 docker image and see if that will reproduce the error.
@effigies - i've narrowed down the issue to the SPM12 matlab r2010a installation in neurodocker 0.4.1. unfortunately i don't have time to dig into it right this minute, but i will this evening / tomorrow morning. matlab has a lot of old shared libraries, so they might be interfering in some way... |
The libraries for Matlab compiler runtime 2010a are being found before the system libraries, and flameo was linking to one of matlab's libraries. This commit prepends the system libraries to LD_LIBRARY_PATH so that flameo (and other binaries) do not link to Matlab's libraries.
thanks @miykael - this indeed was the issue. the most recent circleci build should pass now (it did on my computer). |
Sweet! Here we go. |
Wahoo! And as an added bonus, it takes about 9 minutes to build both the base docker image and the main (py36) image. according to @miykael, the matlab compiler runtime file for matlab 2010a is much smaller than that for 2018b, so using 2010a decreases the spm/mcr installation significantly. that and afni being installed from neurodebian. |
Exactly. Switching to SPM-r7219 decreases MATLAB by a few GB and even SPM by a fe 100MB. And afni (and ants) from neuredebian has a similar reduction effect. Like this I was able to get the tutorial from 12GB to 6GB. |
Summary
This PR proposes updates to Dockerfile generation.
References #2706 (comment)
List of changes proposed in this PR (pull-request)
neurodocker generate
commands.Acknowledgment