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

Failure to build vignette #69

Closed
wleoncio opened this issue Feb 17, 2021 · 7 comments · Fixed by #70
Closed

Failure to build vignette #69

wleoncio opened this issue Feb 17, 2021 · 7 comments · Fixed by #70
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation package Directly related to the R package

Comments

@wleoncio
Copy link
Member

MRE

devtools::install_github("ocbe-uio/cellmigRation", "989fa211")
devtools::build_vignettes()

Expected output

A clean vignette build.

Obtained output

Building cellmigRation vignettes
--- re-buildingcellmigRation.Rmdusing rmarkdown
Bioconductor
  version
  3.12
  (BiocManager
  1.30.10), R
  4.0.4
  (2021-02-15)
Installing
  package(s)
  'cellmigRation'
Warning: package 'cellmigRation' is not available for this version of R

A version of this package for your version of R might be available elsewhere,
see the ideas at
https://cran.r-project.org/doc/manuals/r-patched/R-admin.html#Installing-packages
Computing centroid positionsTotal number of unique cells across images: 0; Cells retained after filtering: 0
Quitting from lines 245-253 (cellmigRation.Rmd) 
Error: processing vignette 'cellmigRation.Rmd' failed with diagnostics:
unable to find an inherited method for function 'setTrackedCentroids' for signature '"trackedCells", "NULL"'
--- failed re-buildingcellmigRation.RmdSUMMARY: processing the following file failed:cellmigRation.RmdError : Vignette re-building failed.
Error: callr subprocess failed: Vignette re-building failed.
Type .Last.error.trace to see where the error occured

System info

> sessionInfo()                                                               
R version 4.0.4 (2021-02-15)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: EndeavourOS

Matrix products: default
BLAS:   /usr/lib/libblas.so.3.9.0
LAPACK: /usr/lib/liblapack.so.3.9.0

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

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

loaded via a namespace (and not attached):
[1] compiler_4.0.4
@wleoncio wleoncio added bug Something isn't working documentation Improvements or additions to documentation labels Feb 17, 2021
@wleoncio
Copy link
Member Author

Summary

When trying to generate the vignette, R creates an R script containing the code chunks up until an error is found. This might provide some insight on where the problem is.

MRE

I've slightly modified the script to achieve minimal reproducibility. Here it is:

set.seed(1234)

library(ggplot2)
library(dplyr)
library(cellmigRation)
library(kableExtra)

data(ThreeConditions)
print(ThreeConditions[[1]])

x1 <- OptimizeParams(
    ThreeConditions$ctrl01, threads = 2, lnoise_range = c(5, 12),
    diameter_range = c(16, 22), threshold_range = c(5, 15, 30),
    verbose = FALSE, plot = TRUE
)
print(x1)
print(getOptimizedParams(x1))

Obtained output

 ~~~ An S4 trackedCells object ~~~

      + Num of images: 7
      + Optimized Params: No
      + Run w/ Custom Params: No
      + Cells Tracked: No
      + Stats Computed: No

$auto_params
NULL

$results
NULL

Expected output

If I understand the problem correctly, the problem is that getOptimizedParams(x1) is NULL on my machine, which is what causes the next chunk (lines 245-253 of cellmigRation.Rmd) to fail. I guess x1 <- CellTracker(tc_obj = x1, min_frames_per_cell = 3, threads = 1, verbose = TRUE) is expecting x1 not to be NULL.

@wleoncio wleoncio self-assigned this Feb 17, 2021
@wleoncio
Copy link
Member Author

Adding myself as an assignee because this could also be a local problem I'll try to debug while @dami82 works on his end.

@dami82
Copy link
Collaborator

dami82 commented Feb 18, 2021

Actually, I double-checked on my laptop, and I cannot reproduce the issue. It looks like the OptimizeParams() function is just not working at all on your system (no issues on mine). Any chance your parallelization is failing? Can you try to set threads = 1 and re-run?

I decided to have a closer look into this by using a virgin environment, so I deployed a small Ubuntu 20.04 instance (a t2.xlarge) on AWS. The OS comes with no R, so here I am installing everything from scratch. In the end, there were NO issues with installation or vignette building.

  • Q: Is it possible that you have not updated some of the libraries (maybe foreach or parallel) when you upgraded to the latest R version?

On AWS/EC2, I used the following code:

# Install R
sudo add-apt-repository 'deb https://cloud.r-project.org/bin/linux/ubuntu focal-cran40/'
sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys E298A3A825C0D65DFD57CBB651716619E084DAB9
sudo apt update
sudo apt install r-base r-base-core r-recommended r-base-dev -y

# Install required libs
sudo apt-get install libssl-dev libcurl4-openssl-dev -y
sudo apt-get install libxml2-dev libtiff-dev -y
sudo apt-get install libfontconfig1-dev libcairo2-dev -y

# Install devtools and some requires R libs
mkdir R
Rscript -e "install.packages(c('usethis', 'roxygen2', 'httr', 'devtools'), lib = '~/R')"
Rscript -e "install.packages(c('tiff', 'vioplot', 'kableExtra'), lib = '~/R')"

# Don't forget to set folder with downloaded libs
echo ".libPaths('~/R')" > .Rprofile

# Install cellmigRation
# No pdflatex, no manual
Rscript -e "devtools::install_github('ocbe-uio/cellmigRation', ref='989fa211', force= TRUE, dependencies = TRUE, build_manual = FALSE, build_vignettes = TRUE, lib = '~/R')"

## ✔  checking for file ‘/tmp/RtmpRNEXe7/remotes9a5b277abab8/ocbe-uio-cellmigRation-989fa21/DESCRIPTION’ ...
## ─  preparing ‘cellmigRation’:
## ✔  checking DESCRIPTION meta-information ...
## ─  installing the package to build vignettes
## ✔  creating vignettes (2m 27.4s)
## ─  checking for LF line-endings in source and make files and shell scripts
## ─  checking for empty or unneeded directories
## ─  building ‘cellmigRation_0.99.4.tar.gz’
##    
## * installing *source* package ‘cellmigRation’ ...
## ** using staged installation
## ** R
## ** data
## *** moving datasets to lazyload DB
## ** inst
## ** byte-compile and prepare package for lazy loading
## Warning: no DISPLAY variable so Tk is not available
## ** help
## *** installing help indices
## ** building package indices
## ** installing vignettes
## ** testing if installed package can be loaded from temporary location
## Warning: no DISPLAY variable so Tk is not available
## ** testing if installed package can be loaded from final location
## Warning: no DISPLAY variable so Tk is not available
## ** testing if installed package keeps a record of temporary installation path
## * DONE (cellmigRation)

# Re-build vignette
git clone --branch develop https://github.com/ocbe-uio/cellmigRation.git
cd cellmigRation
Rscript -e "devtools::build_vignettes()"
## 
## ✔ Creating 'doc/'
## ✔ Setting active project to '<no active project>'
## ✔ Setting active project to '<no active project>'
## Moving cellmigRation.html, cellmigRation.R to doc/
## Copying cellmigRation.Rmd to doc/
## ✔ Creating 'Meta/'
## ✔ Setting active project to '<no active project>'
## ✔ Setting active project to '<no active project>'
## Building vignette index

I downloaded the html, it looks exactly as expected.

@wleoncio
Copy link
Member Author

wleoncio commented Feb 18, 2021

Thank you for taking another look at it, Damiano. I think we're quite close to solving this.

I've reran OptimizeParams() with verbose=TRUE to aid debugging (as I should have done long ago 🤦🏽‍♂️) and this is the output from the function:

Testing 9 combination(s) of params.
This may take some time.
Processing starting worker pid=168842 on localhost:11107 at 14:39:24.782
starting worker pid=168841 on localhost:11107 at 14:39:24.784
Error in unserialize(node$con) : error reading from connection
Calls: <Anonymous> -> workLoop -> makeSOCKmaster
Execution halted
Error in unserialize(node$con) : error reading from connection
Calls: <Anonymous> -> workLoop -> makeSOCKmaster
Execution halted

When I set threads=1, the output is clean and x1 is populated as expected. I've run update.packages() and found no packages to update, so version differences shouldn't be to blame. I've also tried as many as 10 threads, no dice. RAM doesn't seem to be a problem, as it doesn't even budge (and is way below capacity on my 16 GB desktop).

I've searched for this "Error in unserialize(node$con) : error reading from connection" error; couldn't find any solution that didn't involve changing the function code itself, which is something we shouldn't be meddling with at this point in time (especially for a non-reproducible issue) unless we're confident in what we're changing.

I've changed all instances of threads in the vignette to 1 and now the whole thing runs (🎉). There are a few warnings (see this gist) which I don't see in your post above, but at least the HTML produced looks OK. A devtools::check() also finishes with no errors, warnings or notes.

Would you consider reviewing a PR from me proposing this threads=1 change on the vignette? This could be a problem that pops up on some other testing environments setup like mine, and the toy dataset seems small enough to be handled by 1 thread on consumer-grade CPUs. I could add a short comment suggesting the user to increase that number if they wish.

@dami82
Copy link
Collaborator

dami82 commented Feb 18, 2021 via email

@wleoncio wleoncio linked a pull request Feb 18, 2021 that will close this issue
@wleoncio
Copy link
Member Author

I may send you a list of short tests that you may run on your laptop to identify where the issue is!

Sure thing, send away!

@wleoncio wleoncio added the package Directly related to the R package label Feb 24, 2021
@dami82
Copy link
Collaborator

dami82 commented Mar 3, 2021

Vignette building seems to work consistently now. Closing the issue...

@dami82 dami82 closed this as completed Mar 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation package Directly related to the R package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants