igrabski / sc-SHC

Significance analysis for clustering single-cell RNA-sequencing data
87 stars 10 forks source link

Replace mclapply with OS-agnostic alternatives #7

Open ATpoint opened 1 year ago

ATpoint commented 1 year ago

Hi, just saw the package on Twitter / the Nature paper and started playing with it a bit. One thing that caught my eye was that the defaults of testClusters() will result in an error on Windows machines as it uses mclapply with > 1 core. My suggestion would be to replace mclapply with either bplapply from BiocParallel to allow the user to register cores appropriately depending on OS (BPPARAM objects), or switch to any other parallelization function/package that supports Windows. I guess in case you want to submit this to CRAN or Bioc at some point it is in any case advisable to use an "OS-agnostic" rather than mclapply parallelization. Hth.

mschilli87 commented 1 year ago

Alternatively, using the future package might be an option. Though it is possible to to register future with BPPARAM via DoparParam.

igrabski commented 1 year ago

Thanks for your comment -- this is very helpful to know! I will look into an OS-agnostic alternative like the ones you suggest and will update appropriately.

lodimk2 commented 5 months ago

Hello,

Just wanted to circle back on this to see if this has been resolved. Is there a recommended alternative to run the testClusters() function on Windows machines?

Appreciate all your help!

igrabski commented 5 months ago

Hi, apologies, I haven't gotten around to this yet! For now, I would recommend sending cores=1 or alternatively parallel=F.

lodimk2 commented 5 months ago

Hello,

This works! Thanks so much.