add Dockerfile and readme for PPP for openmc workflow + fix issue20 #21
Conversation
johnnonweiler
left a comment
There was a problem hiding this comment.
It's good to remove the outdated docker files. There's a lot of stuff in the new docker file that I don't really know about, and the change to ZERO_THRESHOLD is probably good but it looks a bit suspicious. It might be worth talking through some of these changes.
docker/README.md
Outdated
|
|
||
| or drop to an interative bash shell, as ubuntu 20.04 | ||
|
|
||
| ``` |
There was a problem hiding this comment.
it may work on Windows with a X11 server
docker/README.md
Outdated
|
|
||
| NOTE: X11 forward is not working with root user, so do not add this `-e GRANT_SUDO=yes --user root `, perhaps caused by hardcoded absolute home path in `/home/jovyan/.Xauthority` | ||
|
|
||
| `-v /tmp/.X11-unix:/tmp/.X11-unix -e DISPLAY=$DISPLAY -h $HOSTNAME -v $HOME/.Xauthority:/home/jovyan/.Xauthority` |
There was a problem hiding this comment.
explain why those magic options
docker/README.md
Outdated
| `-v /tmp/.X11-unix:/tmp/.X11-unix -e DISPLAY=$DISPLAY -h $HOSTNAME -v $HOME/.Xauthority:/home/jovyan/.Xauthority` | ||
|
|
||
| https://medium.com/@l10nn/running-x11-applications-with-docker-75133178d090 | ||
|
|
There was a problem hiding this comment.
quickly test x11 fwd
docker/README.md
Outdated
|
|
||
| `docker run --rm -p 2222:22 -e GRANT_SUDO=yes --user root -it ppp_openmc bash ` then start the ssh server by `sudo service ssh start ` | ||
|
|
||
| port mapping by `-p 2222:22` |
docker/Dockerfile_ppp_openmc
Outdated
| # build with the following command | ||
| # sudo docker build -f Dockerfile_ppp_openmc -t ppp_openmc . --no-cache | ||
| # build from github source seems possible to cache ( build without --no-cache) if start quickly enough | ||
| # use --no-cache if you know there is no error in this Dockerfile |
There was a problem hiding this comment.
in release , save disk , otherwise hard to reclaim space back
|
John, I will leave you to do the merge, to see if you have been assigned the previledge to do so |
johnnonweiler
left a comment
There was a problem hiding this comment.
Looks good to me. I find the improved comments helpful.
|
|
||
| # why non-root user is needed, because jupyter base image is used, put all files in user home | ||
| # this environment is jupyter/minimal specific | ||
| # add apt option --no-recommended to |
There was a problem hiding this comment.
I don't understand this comment: # add apt option --no-recommended to
| USER $NB_USER | ||
|
|
||
| ###################### dependancies for openmc ################### | ||
| ###################### dependencies for openmc, maob, dagmc ################### |
There was a problem hiding this comment.
Typo: "maob" should be "moab"
| ################### | ||
| # consider to split the docker image from here | ||
| # copy data into docker image could be skipped and use volume map instead | ||
| # but host folder must be mapped to /mat_dir in the container |
There was a problem hiding this comment.
There are two options given here, and what is actually being done is not completely clear. The documentation seems to suggest that the data is currently being included in the Docker file. In the readme file:
Since nuclear material (tendl-2019-hdf5) has been built in, there is no need to map host folder to container like this
However, it looks like the material is not currently being built in - the line cloning the data repository is commented out below.
3 commits
remove all outdated Dockerfiles (keep 1 in the future)
add Dockerfile and readme for PPP for openmc workflow
fix issue20 set a big ZERO_THRESHOLD for goemetry hash calc
Checklist
upstream mainso is mergeable automatically or easily merged even with confliction.git push origin +your_branchto squash some tiny fixes for CI failures