Hail parallelized diff expression - #20
Conversation
| USER root | ||
|
|
||
| RUN adduser $MAMBA_USER sudo \ | ||
| && echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers |
There was a problem hiding this comment.
does this just get run as root? is there a need to have $MAMBA_USER at all?
There was a problem hiding this comment.
the mamba docs very strongly want to use $MAMBA_USER, especially for the conda package installation.
But then there's weirdness where hail batch assumes you run as root, and their internal setup code fails if you don't (I could post an issue with them 🤷 ). So not elegant. You can use anaconda and side step all of this it's just a good amount slower to create the initial docker image (not load in I mean) and I preferred mamba.
The sudo line might not be necessary with the current config, just the USER root line. As a tangent, I was having trouble with hail batch keeping jobs open even after the python finished, because the dask forks/children weren't closing. So I needed mamba to have sudo to do increasingly aggressive kill lines. But then sys.exit worked when I did that
There was a problem hiding this comment.
Oh ok or maybe I just hallucinated.
https://github.com/mamba-org/micromamba-docker
Running with --user=root is supported.
I somehow read that as "is not supported". So maybe it call all be run as root, either way
If rebuilding this image from scratch, the default username mambauser can be adjusted by passing --build-arg MAMBA_USER=new-username to the docker build command. User id and group id can be adjusted similarly by passing --build-arg MAMBA_USER_ID=new-id --build-arg MAMBA_USER_GID=new-gid
There was a problem hiding this comment.
since there's always a USER root command on line 17 and the MAMBA_USER isn't used, I think these lines can just be removed.
There was a problem hiding this comment.
sure ok, do you want me to push a commit removing them?
Just for ease of context switching ease, I can collect the more cosmetic changes and make a bigger commit later, if that works, as long as the bones of the parallelization end up working on your end
There was a problem hiding this comment.
I can do it, was just double-checking that it wasn't doing something important.
| cd /dev/shm | ||
|
|
||
| while true; do | ||
| if compgen -G "/dev/shm/rtmp*" > /dev/null; then | ||
| echo "Some files exist." | ||
| break | ||
| fi | ||
| sleep 30 | ||
| done | ||
|
|
||
| TOTAL=$(ls|grep rtmp|head -n 1|sed -E "s/(^[^_]*_)|(__.*)//g") | ||
| echo $TOTAL | ||
| # Assume divide by 10 | ||
| while true; do ls rtmp*|wc -l; sleep 10; done|tqdm --ncols=100 --update_to --total=$TOTAL --smoothing=0.3 --initial=`ls rtmp* |wc -l` |
There was a problem hiding this comment.
What does this script do? It doesn't look like it's getting used, but maybe you use it interactively?
There was a problem hiding this comment.
Oh sorry, I meant to remove it, it just makes a nice cli progress bar (I used it for the mapping hail). Could be marginally nice if you end up running multiple diffexps per hail batch, just to track the progress, but unecessary
There was a problem hiding this comment.
I guess I just removed it from the Dockerfile, it was called with a background ampersand same as getcpu.sh
| import dask.distributed | ||
|
|
||
| # Maybe faster, equivalent API | ||
| import _pickle as cPickle |
There was a problem hiding this comment.
FYI in python 3 the regular pickle module uses the C implementation already. This difference was relevant in python 2
3 participants
Opening a PR just to keep track of any comments / changes.
This is @JZL's implementation of differential expression via Hail, with the hope that we can massively scale up the computation to remove one of the main blockers in the clustering strategy.