HenrikBengtsson / parallelly

R package: parallelly - Enhancing the 'parallel' Package
https://parallelly.futureverse.org
128 stars 7 forks source link

Wrong order of arguments to sprintf in reporting connection failures #95

Closed kevinushey closed 1 year ago

kevinushey commented 1 year ago

Here:

https://github.com/HenrikBengtsson/parallelly/blob/cb623927372b3c68dc0773be1ab128e423fbe9c0/R/makeClusterPSOCK.R#L260-L263

I believe connectTimeout + 5 should be coming first.

HenrikBengtsson commented 1 year ago

Thank you. Two reproducible examples with different outcomes:

> cl <- parallelly::makeClusterPSOCK(2L, rscript_startup = quote(Sys.sleep(6.0)), connectTimeout = 0.0)
Error in parallelly::makeClusterPSOCK(2L, rscript_startup = quote(Sys.sleep(6)),  : 
  Cluster setup failed (connectTimeout=2.0 seconds). 5 of 2 workers failed to connect.

and

> cl <- parallelly::makeClusterPSOCK(2L, rscript_startup = quote(Sys.sleep(6.0)), connectTimeout = 0.1)
Error in sprintf(ngettext(failed, "Cluster setup failed (connectTimeout=%.1f seconds). %d worker of %d failed to connect.",  : 
  invalid format '%d'; use format %f, %e, %g or %a for numeric objects

Fixed in commit 0e94e251c.

kevinushey commented 1 year ago

Thank you for taking a look so quickly!

HenrikBengtsson commented 1 year ago

Fixed version now on CRAN

kevinushey commented 1 year ago

Thanks again for taking a look so quickly!