Ncpus argument and install2.r about littler HOT 14 CLOSED

It's good. And the double branch is probably also what we need to set options properly. No need to get too cute.

One type as you use two hyphens: --n where it should be one -n. I will clean that up.

from littler.

That sounds like a great idea, and should be pretty simple. Both -n are -p are open. Preferences?

from littler.

My preference would be for -n. What do you think about having a slight tweak to the input? I quite often set Ncpus equal to the number of cores. This would translate to:

• Default: n = 1 -> (Ncpus = 1)
• n = X -> (Ncpus = X)
• n = 0 -> Ncpus = max(1, parallel::detectCores())

from littler.

I like the option of 'maxing out' but I am not sure I like zero for that special value. Hm. Minus one? NA? NULL?

I am indifferent between -n or -p (for "proceses") or even -c (for "cores"). All suitable.

from littler.

I quite like the -c and -1 combination.

Do you have a preference for the default? Either Ncpus = 1 or Ncpus = getOption("Ncpus", 1L) to match install.packages()

from littler.

I like Ncpus = getOption("Ncpus", 1L) as we can sail with the wind of that existing option.

from littler.

Main question now if we call it -c for cores or cpus when we both know it processes. Maybe coroutines ;-)

from littler.

That just occurred to me as I wrote the doc string. Back to -n ?

Also, I'll go for NULL as that matches (sort of) the repo logic

from littler.

Sure.

from littler.

Looks good. Thanks for catching the rogue = which is definitely bad style :)

I am wondering if we should fold the two branches into one. Ie detect "getOption" as you do but then default to detect the system cores as a fallback, ie

if (opt$ncpus == "getOption") {
  # parallel comes with R 2.14+
  opt$ncpus <- getOption("Ncpus", max(1L, parallel::detectCores()))
}

Thoughts? It may well be too aggressive as it would hog all cores unless told otherwise...

Similarly, two lines in the --help text is one more than every other option gets and something I'll probably clean up. Installing Rcpp as an illustration is also weird as that one does not have that many dependencies (ie: none) to benefit from Ncpus which after all only benefits multiple packages at once. Maybe ggplot2 or lme4 or ... ?

from littler.

• Rcpp: I didn't check the number of dep'. Just assumed it was > 1. Changing to ggplot2 makes sense.

• If we fold the branches, we will change the default behaviour of install.packages() and also the current behaviour of install2.r (as you note). I'm happy with this change, and can't imagine why it's bad. To make it less aggressive, we could use max(1, parallel::detectCores() - 1)

• Two lines in help: Yep I didn't like that either, but it's a side effect of having a "special" option.

from littler.

I think that is still too much. We share a big box at work with 36 hyperthreaded cores. If someone uses it there... For example mclapply() does a simple getOption("mc.cores", 2L) giving us 2. install.packages() defaults to 1 unless told otherwise.

We'll make it one line in --help but describe it in the main help body.

from littler.

Updated the PR:

• Rcpp -> ggplot2
• Removed one of the examples in help

from littler.

Also four space indent not two but all minor -- taken care of now.

Thanks again for this. Very nice enhancement.

from littler.