search

mixOmicsTeam / mixOmics

Development repository for the Bioconductor package 'mixOmics '
http://mixomics.org/
153 stars 51 forks source link

PR addressing Issue #214 #218

Closed Max-Bladen closed 1 year ago

Max-Bladen commented 2 years ago

The tune.spls() function did not have any form of BiocParallel implementation which was quite misleading for users. As this function is likely to be more frequently used, this was a priority to resolve.

The main body of the function was adjusted such that now the internal function iter_keep.vals() iterates over all pairs of keepX/keepY. Rather than doing it within each iteration, the function DetermineOptimalKeepVals() is called after all measure (cor, RSS) calculations. Impossible to iteratively build measure.pred using bplapply().

Old function names and structure was preserved to the best of my ability, though some of it sorely needed a face lift

aljabadi commented 2 years ago

Thanks, @Max-Bladen. Great work on spotting and addressing this. I noticed that the checks for consistency of tune(method='spls') and tune.spls are dropped in unit tests. Was there a specific reason for that? Also, why was the RNG specifier removed in unit tests? Thanks

Max-Bladen commented 2 years ago

Hi @aljabadi,

This is something I meant to ask you about but forgot. Due to my implementation of bplapply(), set.seed() is no longer affecting tune.spls(). Hence, calling the method twice on the same seed produced different results - the tune() and tune.spls() methods couldn't be equated. This is also why the tests currently just assess the class of the output and .mixo_rng() was removed. How can I make use of bplapply() without hindering the rng?

Max-Bladen commented 1 year ago

Second set of commits on 13/12/2022 reflect initial set of commits, but replicated after the branch was rebased to master following PR #281.