mschubert / clustermq

R package to send function calls as jobs on LSF, SGE, Slurm, PBS/Torque, or each via SSH
https://mschubert.github.io/clustermq/
Apache License 2.0
146 stars 27 forks source link

fix issue with formating of object size #199

Closed statquant closed 4 years ago

statquant commented 4 years ago

Hello, on my machine

            common_mb = format(private$size_common, units="Mb")

was worth 7700 bytes and the formatting returned NA NA As such the warning was systematically raised The bellow fixes it on my setup

mschubert commented 4 years ago

Thanks!

I'm a bit confused about where this comes from. Does the snipped below also output NA?

os = object.size(rep(1,1e2))
os
format(os, units="Mb")
statquant commented 4 years ago

For me yes

os = object.size(rep(1,1e2))
os
[1] 840 bytes
format(os, units="Mb")
[1] "NA NA"
mschubert commented 4 years ago

This is strange.

I tested on R=4.0.2 and R=3.6.3, and both work correctly for me.

Can you post your sessionInfo()?

edit: And can you check if the issue persists for you with the latest develop? (I'm getting the size from the serialized object now, maybe that makes a difference)

statquant commented 4 years ago

I was using R 3.4.3 in R 4.0.1 the issue is indeed gone

R --vanilla
os = object.size(rep(1,1e2))
os
848 bytes
format(os, units="Mb")
[1] "0 Mb"
mschubert commented 4 years ago

Ok, this seems like a bug in older versions of R.

Note that clustermq only supports R>=3.5.0 as per DESCRIPTION file.

Depends:
    R (>= 3.5.0)

I don't think it makes sense to merge this.

statquant commented 4 years ago

Hello, sorry to come back to this but I actually get the issue with R 4.0

library(clustermq)                                                                                            
partition <- "p_fast_c7"                                                                                      
foo <- function() { return( data.frame(x = 1:10, y = letters[1:10]) ) }                                       
register_dopar_cmq(n_jobs = 5, memory = 40*1024, template = list(partition = partition))                                                                                                          
res <- foreach(i = 1:2) %dopar% foo()    

in the debugger I see

before the change

common_mb = format(private$size_common, units="Mb")
[1] "NA NA"

after the change

common_mb = format(private$size_common, units="MB", standard="SI")       
[1] "0.001 MB"                         
common_mb = as.numeric(regmatches(common_mb, regexpr("^[[:digit:]]+\\.*[[:digit:]]*", common_mb)))
[1] 0.001

The result:

before the change

Warning in (function (...)  :                                                             
  Common data is NA NA. Recommended limit is 1000 (set by clustermq.data.warning option)  
Submitting 2 worker jobs (ID: cmq6378) ...                                                
Running 2 calculations (1 objs/NA NA common; 1 calls/chunk) ...                           
Master: [2.7s 2.1% CPU]; Worker: [avg 43.6% CPU, max 248.4 Mb]                            

after the change

Submitting 2 worker jobs (ID: cmq7715) ...                      
Running 2 calculations (1 objs/NA NA common; 1 calls/chunk) ... 
Master: [11.9s 0.4% CPU]; Worker: [avg 41.8% CPU, max 248.4 Mb] 

I can add another PR if you're ok with it

mschubert commented 4 years ago

Hi @statquant, no need to apologize. If this is still an issue, we should try and understand where it comes from (because I can still not reproduce this)

For the PR itself, I'm not entirely happy to cast NA to as.numeric, as I suspect a better way to address this would be to try and not get NA for memory in the first place.

Can you find out why the example I posted above worked for you, but the one in your last comment did not? What is the difference in the input to format? (I don't see any, but I also don't get NAs)