afids / afids-auto

automatic anatomical fiducial placement
0 stars 1 forks source link

Write Dockerfile #9

Closed tkkuehn closed 2 years ago

tkkuehn commented 2 years ago

This PR adds a dockerfile to the project that can be used to build a docker (and/or singularity container) with afids-auto-train and/or afids-auto-apply.

Note: In testing, I've found a singularity image built from this container to run afids-auto-apply really slowly (specifically the c4d -rf-apply call). I think it still works, but is worth investigating further.

Preemptive comment: It would be good to add a GitHub action to automatically build the container and push it to Dockerhub (something like this). I can add that if we want it for this PR. We may, however, want to discuss making an afids Dockerhub account or something along those lines, though.

tkkuehn commented 2 years ago

Is there an advantage to condensing the RUN into a single command vs splitting it up to be system dependencies, python dependencies, and external tools?

The idea here is to keep the layer count low (see best practices doc), but on review it seems like smart use of multi-stage builds would be better and more readable -- I'll take another look.

Is there an afids dockerhub?

Not that I'm aware of but possibly -- @greydongilmore do you know?

tkkuehn commented 2 years ago

Ah good point, should have documented that somewhere. What you can do with this is build two different containers buy invoking docker build differently:

Then you can push the build containers by their tags.

kaitj commented 2 years ago

In any case, this looks good to me.

One feature that would be nice to have down the line (and probably would replace the current entrypoints), is a command that can select either the train or apply workflows as needed. I think there might be some examples of that in an existing workflow - would have to take a look.

On second thought, that would probably be something where the workflows can be combined and invoked with an appropriate flag down the line if we want, so above comment can be ignored,