precice / fenicsx-adapter

Experimental preCICE-adapter for the open source computing platform FEniCSx
GNU Lesser General Public License v3.0
10 stars 4 forks source link

Updated dockerfile & added release workflow #11

Closed boris-martin closed 2 years ago

boris-martin commented 2 years ago

Changed the dockerfile to install FEniCSx from Ubuntu packages, then install the adapter. I also added a Github Action to push an image on Docker Hub. It would work once 1) A token is set on the repository 2) The PR #1 is merged. Before pushing, I added a rudimentary test: push if and only if python3 -c "import fenicsxprecice" runs without error.

Currently, the adapter is installed in regular mode. It might be interesting to add the -e flag so that this image can be used efficiently for debugging. Opinions welcome!

arvedes commented 2 years ago

Currently, the adapter is installed in regular mode. It might be interesting to add the -e flag so that this image can be used efficiently for debugging. Opinions welcome!

I would rather not install the adapter in editable mode, because I think modifying the adapter stored inside the container is not best practice.

My development workflow (on windows) is to mount a local copy of the repository into the Docker container and install it in editable mode (either manual or using an additional entrypoint.sh script):

docker run -it --rm -v $PWD/:/home/fenicsx-adapter image-name

In this way I am independent of the container (which I remove after closing with the -rm) and have all changes stored locally.

boris-martin commented 2 years ago

So when you start the container you remove the pre-built with pip uninstall then install the mounted one? Or is it enough to install the new one because pip will override the pre-installed version? I like your workflow (I lost some time setting up remote editing through Docker, adding Git credential into new containers etc) so it makes sense, but what's the purpose of pre-installing anything then? Wouldn't you want just a container with FEniCSx and no adapter at this point? Shall we just keep a pre-installed version only for automated testing ?

arvedes commented 2 years ago

I used a container without adapter: https://github.com/nemocrys/dolfinx-openfoam/blob/main/docker/Dockerfile . I'm not sure about the behavior with pre-installed adapter, I'd just try it out.

The version with the pre-installed adapter is definitely useful for testing. Maybe, some people will use it as a reference for installation (at least I do that sometimes ;) ) or directly work with the container in their own projects.

boris-martin commented 2 years ago

I just tested, and installing local version automatically triggers a (clean) uninstall of the pre-installed version. So let's keep the non-editable install, then the user will be free to override it easily.