Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If FST is not installed, r_make fails with Error in assign(dumpto, last.dump, envir = .GlobalEnv) : cannot add bindings to a locked environment #1062

Closed
PedramNavid opened this issue Nov 12, 2019 · 4 comments
Assignees

Comments

@PedramNavid
Copy link

PedramNavid commented Nov 12, 2019

Prework

  • [ X] Read and abide by drake's code of conduct.
  • [X ] Search for duplicates among the existing issues, both open and closed.
  • [ X] Advanced users: verify that the bug still persists in the current development version (i.e. remotes::install_github("ropensci/drake")) and mention the SHA-1 hash of the Git commit you install.

Description

I noticed this when switching from one computer to another. I tried to run the existing plan with r_make but kept getting a strange error message:

Error in assign(dumpto, last.dump, envir = .GlobalEnv) : 
  cannot add bindings to a locked environment

At first, I thought this was related to lock_envir and set that to FALSE but the error remained.
Finally, I tried running my plan with make(plan) and got the error that

Error: package fst not installed. Please install it with install.packages("fst").

Once installing fst on the new computer, the errors went away, as expected.

Reproducible example

Provide a minimal reproducible example with code and output that demonstrates the bug. The reprex package is extremely helpful for this.

Expected result

What should have happened? Please be as specific as possible.

Session info

End the reproducible example with a call to sessionInfo() in the same session (e.g. reprex(si = TRUE)) and include the output.

> sessionInfo()
R version 3.6.1 (2019-07-05)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.3 LTS

Matrix products: default
BLAS/LAPACK: /opt/intel/compilers_and_libraries_2018.2.199/linux/mkl/lib/intel64_lin/libmkl_rt.so

locale:
 [1] LC_CTYPE=C.UTF-8       LC_NUMERIC=C           LC_TIME=C.UTF-8        LC_COLLATE=C.UTF-8    
 [5] LC_MONETARY=C.UTF-8    LC_MESSAGES=C.UTF-8    LC_PAPER=C.UTF-8       LC_NAME=C             
 [9] LC_ADDRESS=C           LC_TELEPHONE=C         LC_MEASUREMENT=C.UTF-8 LC_IDENTIFICATION=C   

attached base packages:
[1] parallel  stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
 [1] rfm_0.2.0         gbm_2.1.5         doFuture_0.8.2    iterators_1.0.12  foreach_1.4.7    
 [6] future_1.15.0     globals_0.12.4    scales_1.0.0.9000 glue_1.3.1        logger_0.1       
[11] DBI_1.0.0         windfallr_0.3.9   R.utils_2.9.0     R.oo_1.23.0       R.methodsS3_1.7.1
[16] recipes_0.1.7     caret_6.0-84      lattice_0.20-38   bigrquery_1.2.0   forcats_0.4.0    
[21] stringr_1.4.0     dplyr_0.8.3       purrr_0.3.3       readr_1.3.1       tidyr_1.0.0      
[26] tibble_2.1.3      ggplot2_3.2.1     tidyverse_1.2.1   drake_7.7.0.9002 

loaded via a namespace (and not attached):
 [1] colorspace_1.4-1          class_7.3-15              rprojroot_1.3-2          
 [4] fs_1.3.1                  rstudioapi_0.10           listenv_0.7.0            
 [7] bit64_0.9-7               prodlim_2019.10.13        lubridate_1.7.4          
[10] xml2_1.2.2                codetools_0.2-16          splines_3.6.1            
[13] doParallel_1.0.15         zeallot_0.1.0             jsonlite_1.6             
[16] broom_0.5.2               compiler_3.6.1            httr_1.4.1               
[19] backports_1.1.5           assertthat_0.2.1          Matrix_1.2-17            
[22] lazyeval_0.2.2            gargle_0.4.0              cli_1.1.0                
[25] prettyunits_1.0.2         tools_3.6.1               igraph_1.2.4.1           
[28] gtable_0.3.0              reshape2_1.4.3            Rcpp_1.0.3               
[31] cellranger_1.1.0          vctrs_0.2.0               nlme_3.1-142             
[34] timeDate_3043.102         googleCloudStorageR_0.5.1 gower_0.2.1              
[37] ps_1.3.0                  rvest_0.3.5               lifecycle_0.1.0          
[40] googleAuthR_1.1.1         MASS_7.3-51.4             ipred_0.9-9              
[43] hms_0.5.2                 yaml_2.2.0                curl_4.2                 
[46] memoise_1.1.0             gridExtra_2.3             rpart_4.1-15             
[49] stringi_1.4.3             filelock_1.0.2            zip_2.0.4                
[52] lava_1.6.6                storr_1.2.1               rlang_0.4.1              
[55] pkgconfig_2.0.3           bit_1.1-14                processx_3.4.1           
[58] tidyselect_0.2.5          here_0.1                  plyr_1.8.4               
[61] magrittr_1.5              R6_2.4.0                  generics_0.0.2           
[64] base64url_1.4             txtq_0.2.0                pillar_1.4.2             
[67] haven_2.2.0               withr_2.1.2               survival_2.44-1.1        
[70] nnet_7.3-12               modelr_0.1.5              crayon_1.3.4             
[73] progress_1.2.2            grid_3.6.1                readxl_1.3.1             
[76] data.table_1.12.6         callr_3.3.2               ModelMetrics_1.2.2       
[79] digest_0.6.22             openssl_1.4.1             stats4_3.6.1             
[82] munsell_0.5.0             askpass_1.1       
@wlandau
Copy link
Member

wlandau commented Nov 12, 2019

Just to confirm, I can indeed reproduce what you see.

library(fst)
#> Error in library(fst): there is no package called 'fst'
library(drake)
writeLines(
  c(
    "library(drake)",
    "plan <- drake_plan(x = target(mtcars, format = \"fst\"))",
    "config <- drake_config(plan)"
  ),
  "_drake.R"
)
r_make()
#> �[32mtarget�[39m x
#> Error in assign(dumpto, last.dump, envir = .GlobalEnv) : 
#>   cannot add bindings to a locked environment

Created on 2019-11-12 by the reprex package (v0.3.0)

@wlandau
Copy link
Member

wlandau commented Nov 12, 2019

r_make() is built on callr. When a callr process throws an error, it dumps the error and stacktrace to the global environment (variables Last.error etc). When I manually unlocked the global environment right before throwing the error in assert_pkg(), the error message became more informative.

library(callr)
library(drake)
writeLines(
  c(
    "library(drake)",
    "plan <- drake_plan(x = target(mtcars, format = \"fst\"))",
    "config <- drake_config(plan)"
  ),
  "_drake.R"
)
r_make()
#> �[32mtarget�[39m x
#> Error : package fst not installed. Please install it with install.packages("fst").
#> Error: callr subprocess failed: package fst not installed. Please install it with install.packages("fst").

Created on 2019-11-12 by the reprex package (v0.3.0)

Perhaps r_make() needs to always lock/unlock the environment separately for each target. Or maybe drake should do this for make() too. Need to revisit profiling studies to make sure this is fast enough.

@wlandau
Copy link
Member

wlandau commented Nov 12, 2019

Checked profiling studies, and unlocking/relocking the environment did not even put a dent in performance. So I think that's the right way to go. It will allow these informative error messages to appear without having to set lock_envir to FALSE.

@wlandau
Copy link
Member

wlandau commented Nov 12, 2019

Should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants