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

CI: Attempt to test all scripts on merge to master #1

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

Lestropie
Copy link
Owner

Adds a Dockerfile for building a container tailored for executing all script tests, including those with external neuroimaging software dependencies. An additional workflow is then intended to build and execute that container only for merges to master.

@Lestropie Lestropie self-assigned this Sep 3, 2020
@Lestropie
Copy link
Owner Author

Note to self: Currently the CI will have to download & install the whole system, in addition to fixed versions of ANTs and FSL, for every CI test. An alternative would have to be a pre-built container based on an additional repository, which includes all of these contents: this repository would then simply specify this container in the Dockerfile recipe. The disadvantange would be that updates to other neuroimaging softwares would not immediately be imported; but this would simply require updating and rebuilding that template container.

While the CI action execution has not yet reached this point (the job appears to fail during building of the container, but no useful error message is produced), I nevertheless do not think that such a Checkout operation should be necessary here; all that is required is that the relevant commitish be passed as an argument to the container at the point of execution.
@Lestropie
Copy link
Owner Author

  Step 10/16 : RUN wget -q http://fsl.fmrib.ox.ac.uk/fsldownloads/fslinstaller.py -O /fslinstaller.py     && chmod 775 /fslinstaller.py     && python2 /fslinstaller.py -d /opt/fsl -V 6.0.4 -q     && rm -f /fslinstaller.py     && ( which immv || ( rm -rf /opt/fsl/fslpython && /opt/fsl/etc/fslconf/fslpython_install.sh -f /opt/fsl || ( cat /tmp/fslpython*/fslpython_miniconda_installer.log && exit 1 ) ) )
   ---> Running in 40f6e1aec579
  [FAILED] Unable to unpack FSL - Unable to unpack FSL.
  The command '/bin/sh -c wget -q http://fsl.fmrib.ox.ac.uk/fsldownloads/fslinstaller.py -O /fslinstaller.py     && chmod 775 /fslinstaller.py     && python2 /fslinstaller.py -d /opt/fsl -V 6.0.4 -q     && rm -f /fslinstaller.py     && ( which immv || ( rm -rf /opt/fsl/fslpython && /opt/fsl/etc/fslconf/fslpython_install.sh -f /opt/fsl || ( cat /tmp/fslpython*/fslpython_miniconda_installer.log && exit 1 ) ) )' returned a non-zero code: 1
  ##[warning]Docker build failed with exit code 1, back off 8.871 seconds before retry.
  /usr/bin/docker build -t 3b3ac6:84564b29c60e43bfb8327824a5fd2027 -f "/home/runner/work/_actions/MRtrix3/script_test_action/master/Dockerfile" "/home/runner/work/_actions/MRtrix3/script_test_action/master"
  Sending build context to Docker daemon  8.704kB
  
  Error response from daemon: Error processing tar file(exit status 1): write /action.yml: no space left on device

:-/

@Lestropie
Copy link
Owner Author

Container grabs code, compiles and starts tests, but they get terminated after 6 hours.
So will need to pursue test data drastically reduced in spatial resolution to complete more quickly, and/or Daan's suggestion of replacing external software commands with dummies that dump pre-generated data; but given population_template is one of the culprits, reduction in spatial resolution is probably going to be needed regardless.

- Import new dwifslpreproc data that has been further reduced in terms of resolution and number of volumes.
- Specify the number of iterations for topup and eddy to be 1 and 0 respectively.
- New dwifslpreproc command_line option "-topup_config" that allows a custom configuration file to be passed to topup; including "--config=" in the input via -topup_options is also supported.
@Lestropie
Copy link
Owner Author

@maxpietsch: I've massively cut down on dwifslpreproc test run times trying to make CI tests with little to no multi-threading and 6 hour runtime feasible (and hopefully actually add more tests to verify all of its features). Ideally I'd like to cut down on population_template substantially too since that's the other primary culprit. There's a few things we could do e.g.:

  • Reduce spatial resolution;

  • Reduce the number of tests that are done comprehensively looking for regressions in output data; for those that only need to be run to verify completion given command-line options, we could cut number of stages / number of iterations within each stage.

Though CI is actually out of time before the tests even reach that point, so there needs to be further cuts elsewhere...

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.

1 participant