WIP: add tests/refactor fieldmap code

[berleant]

No description provided.

[oesteban]

@shoshber, maybe this would fit in this PR since you are refactoring the fieldmaps machinery.

The naming se_pair_workflow happens to be incorrect. Actually, in the AA database, the fieldmap is computed from 8 images (four pairs of SE acquisitions). We should think of a different way to name all of this because pair is particularly misleading: it is easily confounded with computing the fieldmap using a pair of GRE acquisitions (#20) and looking at the phase difference. Actually, the latter case is a lot more appropriate for the pair word, since it is always computed with two (you could use more, but I don't think anybody is doing that).

On the other case (our current se_pair_workflow) it is appropriate to have more than two images. Results improve significantly.

[oesteban]

Actually, this was already open: #62

[berleant]

Circle ci test is failing because the docker image does not rebuild unless preprocessing-workflow/master is changed, but it needs to be rebuilt to read the new dependencies.

[oesteban]

Circle ci test is failing because the docker image does not rebuild unless preprocessing-workflow/master is changed, but it needs to be rebuilt to read the new dependencies.

Are you sure? I'd say you are not not using the docker image to run your tests... But I might be wrong because I just took a very brief look into this.

[oesteban]

This is not using the docker image to run your tests... Please see build/circle_tests.sh to see how to run them inside the docker image.

[berleant]

thank you! I had not looked into this very much but I think you just saved me a lot of time

[oesteban]

the workflow is containerized in a docker image. Thus, anything you want to do should be inside the image. To run code inside the image there are several strategies (see https://github.com/poldracklab/preprocessing-workflow/blob/master/build/circle_tests.sh#L6).

Basically, you may want to write an entry point for the docker image (like https://github.com/poldracklab/preprocessing-workflow/blob/master/build/files/run_fmriprep) in which you source the environment and call the tests. Finally, you will call the fmriprep docker image setting this new entrypoint (with something like --entrypoint=/usr/local/bin/run_shoshber_tests). This will replace the default entrypoint run_fmriprep).

Anything you place in that directory with the preffix run_ will be placed and granted permissions in the docker image automatically.

[oesteban]

You are almost there. Before the image name oesteban/fmriprep you need to specify the entrypoint. In your case it is --entrypoint=/usr/bin/run_unittests. And you're done!

Edit by @shoshber: I think this is already true?

[oesteban]

BTW, if you want CircleCI to understand this test as a separate test (and run always the second line of this file), then you will need to put this line as a new entry in the circle.yml file. Actually, having this file is not necessary here, we could also get the smoke test out to the yml file and remove this build/circle_tests.sh

[berleant]

Great suggestion! I won't act on it now because not trying to find a perfect solution here, especially because I'm supposed to be working on fieldmap stuff--I just want to close this pull request because long-running pr's are bad form in my book. It is true that the test infrastructure (e.g., files in the build directory) in this package is a little wonky. Maybe create an issue for un-wonkifying the test infrastructure?

[oesteban]

@shoshber Ok, now I see that the entrypoint was there :). Sorry about that.

[berleant]

Yussssssss