Notame normalization and feature selection #6
Conversation
There was a problem hiding this comment.
This is really good ! thank you so so much for taking the time for this. I have added comments for things that need to be updated.
A general comment would be that if you have parameters specific to notame functions maybe detail them a bit in the text. otherwise I find it very well described.
I will update the trunk vignette asap so that the SummarizedExperiment is saved and therefore your vignette can work with everything else !
Request my feedback (button on the top right) when you have done the little fixes.
| library(notame) | ||
| library(SummarizedExperiment) | ||
|
|
||
| registerDoParallel(cores = 14) |
There was a problem hiding this comment.
For the other vignettes we use the BiocParallel package for setting parallelisation. Also we use 2 cores :) So the code should be as below:
#' Set up parallel processing using 2 cores
if (.Platform$OS.type == "unix") {
register(MulticoreParam(2))
} else {
register(SnowParam(2))
}
There was a problem hiding this comment.
and therefore no need for the doParallel package
There was a problem hiding this comment.
Actually, I have a question here @vsuksi - I assume you use doParallel parallel processing code within the notame package? Also thinking on the Bioconductor submission of your package, I think it would be good to change/switch here to BiocParallel parallel processing functions (i.e. bplapply(), bpmapply() etc). The BiocParallel is the standard package in Bioconductor to register/configure parallel processing. So, something like the example code above from Phili would then be used across all used packages for the parallel processing. What I found particularly helpful with BiocParallel is that it allows to use a variety of different configurations, from SerialParam to disable parallel processing, to SnowParam (for Windows) MulticoreParam (for Unix) to also parameters that allow to configure and use HPC queueing systems like slurm. And there is also a param for doParallel too :)
So, maybe, on the longer run, it would make sense to switch the parallel processing code within notame to BiocParallel?
There was a problem hiding this comment.
Good to hear and sorry for the delay, I need to practice juggling several things at once! Happily the Bioconductor-version of notame uses BiocParallel instead of doParallel, I will switch to that version and the code chunk above after we're done with some larger changes and submit. We've been taking some time to make this the first real release, but I'm hoping it is ready in a week or two.
|
Just converting to draft so that we don't merge per error until it's ready :) |
…n_and_feature_selection. Just keeping in sync.
Hi, I think I've addressed your suggestions, thanks! I also changed some minor things and clarified in the normalization visualizations what one would typically look for.
|
Hi, I think I've addressed your suggestions, thanks! I also changed some minor things and clarified in the normalization visualizations what one would typically look for. |
| @@ -0,0 +1,42 @@ | |||
| @article{klavus_notame_2020, | |||
There was a problem hiding this comment.
This is perfect thanks a lot !
|
Hi @vsuksi , Indeed this is completely good now ! If the next version of notame with Thanks a lot again ! Super excited to add it to the website :) |
|
|
||
| registerDoParallel(cores = 14) | ||
|
|
||
| dir <- system.file("extdata", "preprocessed_res", package = "metabonaut") |
There was a problem hiding this comment.
Hi sorry just tiny thing that needs to change: can you write Metabonaut with a capital letter instead of metabonaut
…n_and_feature_selection
Now uses SummarizedExperiment. Flagged features are carried further. Noticed that I used the non gap-filled assay for the analysis, rectified that (not much changed). Clarified some text.
Initial version, using notame 0.3.2, https://github.com/antonvsdata/notame.