Nik-Zainal-Group / signature.tools.lib

R package containing useful functions for mutational signature analysis
Other
80 stars 26 forks source link

does not work with Windows? #29

Closed ahwanpandey closed 3 years ago

ahwanpandey commented 3 years ago

Hello,

Thanks for this tool!

I am trying to install this on windows but got this error:

image

seems the doMC package is not available for windows. Does this mean the "signature.tools.lib" is not compatible with windows?

Thanks, Ahwan

ahwanpandey commented 3 years ago

I've been able to install the package by removing the "doMC" Import in the DESCRIPTION.

Also was able to run EXAMPLE1 all the way to HRDetect by commenting out the doMC code in HRDetect.R #doMC::registerDoMC(nparallel)

So it does seem to run with a few tweaks, just not by default?

andreadega commented 3 years ago

Thanks for your bug report. I am currently away and back in September, but I can guess that the problem may have to do with the fact that the doMC function doesn't actually run things in parallel in Windows, so probably some repositories do not make it available.

Once I am back I will check the code and see if doMC can be removed in favour of other parallel frameworks that are compatible with Windows.

Andrea

On Fri, 27 Aug 2021, 00:29 ahwanpandey, @.***> wrote:

I've been able to install the package by removing the "doMC" Import in the DESCRIPTION.

Also was able to run EXAMPLE1 all the way to HRDetect by commenting out the doMC code in HRDetect.R

doMC::registerDoMC(nparallel)

So it does seem to run with a few tweaks, just not by default?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Nik-Zainal-Group/signature.tools.lib/issues/29#issuecomment-906785722, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADTMMA6UZC34W2UZ6M3FJODT6253FANCNFSM5C4IKFSQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

ahwanpandey commented 3 years ago

Hi, thanks for your reply! I have managed to run most of the functions now without multithreading on a Windows machine after the above mentioned changes.

ahwanpandey commented 3 years ago

Same error with the SignatureExtraction function

extract_res = SignatureExtraction(
+   outFilePath = output_folder,
+   cat = category_counts_mat,
+   matrix_of_fixed_signatures = reference_signature_mat,
+   nboots = 20,
+   nrepeats = 200,
+   clusteringMethod = "MC",
+   nmfmethod = "brunet",
+   filterBest_RTOL=0.001
+ )
Loading required package: pkgmaker
Loading required package: registry
Loading required package: rngtools
Loading required package: cluster
NMF - BioConductor layer [OK] | Shared memory capabilities [NO: windows] | Cores 7/8
Error in loadNamespace(name) : there is no package called ‘doMC’
ahwanpandey commented 3 years ago

Seems SignatureExtraction is the slowest without parallelizing :(

andreadega commented 3 years ago

Our signature extraction framework, which includes selecting only the best NMF runs, requires a lot of NMF runs (we suggest to set 20 bootstraps and at least 100-200 repeats for each bootstrap for a given number of signatures), so using it without parallelisation can be quite difficult for a problem with hundreds of samples.

Currently we are working on a major new release of the signatures.tools.lib package, which is quite some time away, so I will probably not upload the fix soon. However I can suggest you what line of code to edit manually to fix it, once I am back to work next week.

Thanks for your patience. Andrea

On Wed, 1 Sep 2021, 10:58 ahwanpandey, @.***> wrote:

Seems SignatureExtraction is the slowest without parallelizing :(

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Nik-Zainal-Group/signature.tools.lib/issues/29#issuecomment-910082295, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADTMMA5LRZADUIFN4BIR3KTT7XTNBANCNFSM5C4IKFSQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

andreadega commented 3 years ago

As a quick fix for Windows parallelisation, until the new release, please replace all instances of:

doMC::registerDoMC(nparallel)

with

doParallel::registerDoParallel(nparallel)

This code should be replaced in 4 files:

ahwanpandey commented 3 years ago

Thanks @andreadega. Just tested and working for SignatureFit_withBootstrap and SignatureExtraction