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

Loading cytolib crashes mbkmeans #45

Open
LTLA opened this issue Jan 8, 2021 · 6 comments
Open

Loading cytolib crashes mbkmeans #45

LTLA opened this issue Jan 8, 2021 · 6 comments

Comments

@LTLA
Copy link

LTLA commented Jan 8, 2021

In the same vein as #37 and #38, I get:

library(cytolib)
library(mbkmeans)
set.seed(1000)
x <- matrix(runif(10000), ncol=10)
out <- mbkmeans(t(x), 20)
## error: arma::memory::acquire(): out of memory
## Error in mini_batch(data = t(x), clusters = clusters, batch_size = batch_size,  : 
##   std::bad_alloc

This fails 100% of the time on my system. I am confident that mbkmeans is not doing any particularly unusual. The more obvious culprit is the local=FALSE in cytolib's dyn.load statement; setting it back to TRUE avoids this error.

Session information
R Under development (unstable) (2021-01-01 r79759)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.5 LTS

Matrix products: default
BLAS:   /home/luna/Software/R/trunk/lib/libRblas.so
LAPACK: /home/luna/Software/R/trunk/lib/libRlapack.so

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

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

other attached packages:
[1] mbkmeans_1.7.1 cytolib_2.3.0 

loaded via a namespace (and not attached):
 [1] gmp_0.6-2                   Rcpp_1.0.5                 
 [3] benchmarkmeData_1.0.4       bluster_1.1.2              
 [5] XVector_0.31.1              GenomeInfoDb_1.27.3        
 [7] pillar_1.4.7                compiler_4.1.0             
 [9] BiocNeighbors_1.9.4         iterators_1.0.13           
[11] zlibbioc_1.37.0             bitops_1.0-6               
[13] tools_4.1.0                 MatrixGenerics_1.3.0       
[15] SingleCellExperiment_1.13.3 lifecycle_0.2.0            
[17] tibble_3.0.4                gtable_0.3.0               
[19] lattice_0.20-41             pkgconfig_2.0.3            
[21] rlang_0.4.10                igraph_1.2.6               
[23] foreach_1.5.1               Matrix_1.3-2               
[25] DelayedArray_0.17.7         parallel_4.1.0             
[27] GenomeInfoDbData_1.2.4      httr_1.4.2                 
[29] dplyr_1.0.2                 generics_0.1.0             
[31] vctrs_0.3.6                 S4Vectors_0.29.6           
[33] gtools_3.8.2                IRanges_2.25.6             
[35] stats4_4.1.0                grid_4.1.0                 
[37] tidyselect_1.1.0            Biobase_2.51.0             
[39] glue_1.4.2                  R6_2.5.0                   
[41] BiocParallel_1.25.2         RProtoBufLib_2.3.0         
[43] ggplot2_3.3.3               purrr_0.3.4                
[45] ClusterR_1.2.2              magrittr_2.0.1             
[47] codetools_0.2-18            GenomicRanges_1.43.1       
[49] scales_1.1.1                ellipsis_0.3.1             
[51] matrixStats_0.57.0          BiocGenerics_0.37.0        
[53] SummarizedExperiment_1.21.1 benchmarkme_1.0.4          
[55] colorspace_2.0-0            doParallel_1.0.16          
[57] RCurl_1.98-1.2              RcppParallel_5.0.2         
[59] munsell_0.5.0               crayon_1.3.4               
@gfinak
Copy link
Member

gfinak commented Jan 15, 2021

@LTLA thanks for the report.
Given that this has come up three times, we're working on a fix. We'll build cytolib as a static library and have packages that link against it link it statically.

@mikejiang
Copy link
Member

Just a note for myself (as a reference later), after building cytolib as static lib, flowCore and flowWorkspace link and run fine. But CytoML fails to load

** testing if installed package can be loaded from temporary location
[libprotobuf ERROR google/protobuf/descriptor_database.cc:644] File already exists in database: GatingSet.proto
[libprotobuf FATAL google/protobuf/descriptor.cc:1371] CHECK failed: GeneratedDatabase()->Add(encoded_file_descriptor, size): 
terminate called after throwing an instance of 'google::protobuf::FatalException'
  what():  CHECK failed: GeneratedDatabase()->Add(encoded_file_descriptor, size): 
Aborted (core dumped)
ERROR: loading failed
* removing ‘/mylib/R-devel-build/library/CytoML’

It is likely caused by the fact that both CytoML and flowWorkspace contains the translation unit GatingSet.pb.cc after they are statically linked to libcytolib.a, which leads to shared protobub lib (from RProtobufLib) error.

The fix is to build RProtobufLib as static lib as well

@mikejiang
Copy link
Member

Building and linking RProtobufLib as static lib resolve the CytoML loading libprotobuf ERROR above.
But running into runtime error

==> devtools::test()

Testing flowWorkspace|  OK F W S | Context
...| 131       | ---- gs [5.1 s]                                                                                                                      
✓ | 119       | ---- gh [0.8 s]                                                                                                                      
⠴ |   0   6   | -- parsed gs                                                                                                                         terminate called after throwing an instance of 'std::system_error'
  what():  Invalid argument

backtrace shows it is still from libprotobuf during loading cytoml (probably conflicts with flowWorkspace)

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7928859 in __GI_abort () at abort.c:79
#2  0x00007ffff54c5951 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff54d147c in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff54d14e7 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff54d1799 in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007ffff54c877f in std::__throw_system_error(int) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#7  0x00007fffeb62f9fc in std::mutex::lock (this=0x555557e944d8) at /usr/include/c++/10/bits/std_mutex.h:104
#8  google::protobuf::internal::WrappedMutex::Lock (this=0x555557e944d8) at ./google/protobuf/stubs/mutex.h:99
#9  google::protobuf::internal::MutexLock::MutexLock (mu=0x555557e944d8, this=<synthetic pointer>) at ./google/protobuf/stubs/mutex.h:118
#10 google::protobuf::internal::OnShutdownRun (f=0x7fffeb6266a0 <google::protobuf::internal::DestroyString(void const*)>, 
    arg=0x7fffebb41e60 <google::protobuf::internal::fixed_address_empty_string[abi:cxx11]>) at google/protobuf/message_lite.cc:566
#11 0x00007fffeb626c7e in google::protobuf::internal::OnShutdownDestroyString (ptr=0x7fffebb41e60 <google::protobuf::internal::fixed_address_empty_string[abi:cxx11]>)
    at ./google/protobuf/generated_message_util.h:251
#12 google::protobuf::internal::InitProtobufDefaultsImpl () at google/protobuf/generated_message_util.cc:76
#13 google::protobuf::internal::InitProtobufDefaults () at google/protobuf/generated_message_util.cc:81
#14 0x00007fffeb6272d8 in google::protobuf::internal::InitSCCImpl (scc=0x7fffebb3acc0 <scc_info_BOOL_GATE_OP_GatingSet_2eproto>) at google/protobuf/generated_message_util.cc:815
#15 0x00007fffeb659c3d in google::protobuf::internal::InitSCC (scc=<optimized out>) at ./google/protobuf/generated_message_util.h:240
...
#24 0x00007ffff7fe45fa in _dl_open (file=0x7ffffff4e1e0 "/library/CytoML/libs/CytoML.so", mode=-2147483646, caller_dlopen=<optimized out>, nsid=-2, 

@mikejiang
Copy link
Member

Ok. revert back to shared RProtobuflib, and move GatingSet.pb.cc from cytolib to RProtobuflib and build it into shared lib as well (to avoid multiple instances that leads to libprotobuf error).
Now flowWorkspace and cytoml can coexist .

==> devtools::test()
...|  61       | -- archived gs [0.8 s]                                                                                                               
✓ | 134       | ---- gs [5.3 s]                                                                                                                      
✓ | 122   1   | ---- gh [1.1 s]                                                                                                                      
────────────────────────────────────────────────────────────────────────────────✓ |   2   7   | -- parsed gs [1.8 s]                                                                                                                 
✓ | 119       | ---- gh [1.2 s]                                                                                                                      
✓ | 222   10   | ---- gs [16.1 s]                                                                                                                    
...
[ FAIL 0 | WARN 19 | SKIP 19 | PASS 1675 ]

mikejiang pushed a commit to RGLab/RProtoBufLib that referenced this issue Jan 21, 2021
@mikejiang
Copy link
Member

@LTLA , with the latest patch in master

> library(cytolib)
> library(mbkmeans)

> set.seed(1000)
> x <- matrix(runif(10000), ncol=10)
> out <- mbkmeans(t(x), 20)
> 

also

> 
> library(cytolib)
> library(mzR)
> 

I am also yet to test it on windows.

mikejiang pushed a commit that referenced this issue Jan 22, 2021
mikejiang pushed a commit to RGLab/RProtoBufLib that referenced this issue Jan 22, 2021
mikejiang pushed a commit that referenced this issue Jan 22, 2021
mikejiang pushed a commit to RGLab/RProtoBufLib that referenced this issue Jan 23, 2021
@mikejiang
Copy link
Member

Looks like windows still doesn't like the idea of using shared library from r package, building and exposing GatingSet.pb.cc as dll cause numerous failures in flowWorkspace tests.
Switching to static link for this file seems to pass all the tests without incurring the same protobuf::internal::WrappedMutex::Lock error from linux.

I am closing it now. Feel free to reopen it if some other issues come up related to this.

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

No branches or pull requests

3 participants