search

zamboni-lab / SLAW

Scalable and self-optimizing processing workflow for untargeted LC-MS
GNU General Public License v2.0
25 stars 3 forks source link

Incorrect assignment of processes on CPU cores #24

Closed chufz closed 1 year ago

chufz commented 1 year ago

screenshot-alacritty-2022-12-01-10-11-55 Hi,

I am using the current version of slaw on an HPC cluster (os7) using a singularity container. I am truncating the memory and cores to the applied resources of the job submit by:

SINGULARITYENV_NCORES=8 SINGULARITYENV_MEMORY=4000 singularity run

The output file then states the correct assignment of parameters:

Total memory available: 435561 and 56 cores. The workflow will use 4000 Mb by core on 8 cores. However, I observed that on the nodes more processes were spawned, see attached screenshot. Also, only one of these processes was actually running, the others were blocked on waiting.

I just briefly went through the code and I am wondering if the 'NUM_CORES' variable in the Rscripts (like fusing_msms_spectra.R) are assigned correctly?

Thanks a lot for your help

htmonkey commented 1 year ago

which branch?

chufz commented 1 year ago

In the past, was pulling from docker://zambonilab/slaw:latest, I am now pulling from this github Master branch, however can you tell me which Branch is currently the best to work with/ same code to the DockerHub?

I am also wondering why the MSMS aggregation is performed twice in the current Master Branch ?

fcc <- bpmapply(listidx,listspecs,FUN = mergeSpectraEfficient,BPPARAM=bpp,SIMPLIFY = FALSE)

fcc <- bpmapply(listidx,listspecs,FUN = mergeSpectraEfficient,BPPARAM=bpp,SIMPLIFY = FALSE)

line 591/593 of /pylcmsprocessing/Rscript/fusing_msms_spectra.R

htmonkey commented 1 year ago

slaw:latest on Docker is set to build automatically from the slaw:master branch of the Github repository. The double entry you spotted is an obvious error. I did some housekeeping on the function.

As for parallel processing, the code seems ok. I didn't check how the cores are used, but the correct BPPARAM is propagated accordingly to all functions. I'll keep an eye on it. I am anyway updating/extending/redesigning the output layer, and this is part of it.