kuwisdelu / matter

Out-of-core statistical computing and signal processing
http://www.cardinalmsi.org
54 stars 4 forks source link

[Bug]: NAs not correctly propagated for summary statistics of integer matter_mat #5

Closed PeteHaitch closed 7 years ago

PeteHaitch commented 7 years ago

Hi Kylie,

The summary statistics for matter objects (colSums(), etc.) return incorrect results when NA are present and the datamode of the matter object is "integer":

suppressPackageStartupMessages(library(matter))

y <- matrix(c(1:8, NA_integer_), ncol = 3)
Y_integer <- matter_mat(y, nrow = nrow(y), ncol = ncol(y), datamode = "integer")

colSums(y)
#> [1]  6 15 NA
colSums(Y_integer)
#> [1]           6          15 -2147483633

colSums(y, na.rm = TRUE)
#> [1]  6 15 15
colSums(Y_integer, na.rm = TRUE)
#> [1]           6          15 -2147483633

Everything seems to be fine if the datamode of the matter object is "double":

suppressPackageStartupMessages(library(matter))

y <- matrix(c(1:8, NA_integer_), ncol = 3)
Y_double <- matter_mat(y, nrow = nrow(y), ncol = ncol(y), datamode = "double")

colSums(y)
#> [1]  6 15 NA
colSums(Y_double)
#> [1]  6 15 NA

colSums(y, na.rm = TRUE)
#> [1]  6 15 15
colSums(Y_double, na.rm = TRUE)
#> [1]  6 15 15

Cheers, Pete

kuwisdelu commented 7 years ago

Hi Pete,

I just pushed a commit to Github that should fix this (v1.3.4).

-Kylie

PeteHaitch commented 7 years ago

Hmm, I can't install the current GitHub master, so I can't try it out. I can, however, install the Bioconductor release via both BiocInstaller::biocLite("matter") and devtools::install_github('Bioconductor-mirror/matter')

I tried on two different machines. Does this work for you?

> devtools::install_github('kuwisdelu/matter')
Using GitHub PAT from envvar GITHUB_PAT
Downloading GitHub repo kuwisdelu/matter@master
from URL https://api.github.com/repos/kuwisdelu/matter/zipball/master
Installing matter
'/Library/Frameworks/R.framework/Resources/bin/R' --no-site-file  \
  --no-environ --no-save --no-restore --quiet CMD INSTALL  \
  '/private/var/folders/f1/6pjy5xbn0_9_7xwq6l7fj2yc0000gn/T/RtmpEVzvEN/devtoolscaca132787e3/kuwisdelu-matter-c9f0998'  \
  --library='/Library/Frameworks/R.framework/Versions/3.4/Resources/library'  \
  --install-tests 

* installing *source* package ‘matter’ ...
** libs
/usr/local/clang4/bin/clang++ -I/Library/Frameworks/R.framework/Resources/include -DNDEBUG   -I/usr/local/include   -fPIC  -Wall -g -O2  -c init.cpp -o init.o
/usr/local/clang4/bin/clang++ -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup -single_module -multiply_defined suppress -L/Library/Frameworks/R.framework/Resources/lib -L/usr/local/clang4/lib -o matter.so init.o matter.o utils.o -F/Library/Frameworks/R.framework/.. -framework R -Wl,-framework -Wl,CoreFoundation
installing to /Library/Frameworks/R.framework/Versions/3.4/Resources/library/matter/libs
** R
** data
** tests
** preparing package for lazy loading
Creating a new generic function for ‘apply’ in package ‘matter’
Creating a new generic function for ‘scale’ in package ‘matter’
** help
*** installing help indices
** building package indices
** installing vignettes
** testing if installed package can be loaded
Error: package or namespace load failed for ‘matter’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object '/Library/Frameworks/R.framework/Versions/3.4/Resources/library/matter/libs/matter.so':
  dlopen(/Library/Frameworks/R.framework/Versions/3.4/Resources/library/matter/libs/matter.so, 6): Symbol not found: _countRuns
  Referenced from: /Library/Frameworks/R.framework/Versions/3.4/Resources/library/matter/libs/matter.so
  Expected in: flat namespace
 in /Library/Frameworks/R.framework/Versions/3.4/Resources/library/matter/libs/matter.so
Error: loading failed
Execution halted
ERROR: loading failed
* removing ‘/Library/Frameworks/R.framework/Versions/3.4/Resources/library/matter’
Installation failed: Command failed (1)
kuwisdelu commented 7 years ago

Hi Pete,

Somehow some old object files ended up in the /src directory. They just needed to be removed. I cleaned them up and it should work now.

-Kylie

PeteHaitch commented 7 years ago

Thanks for the quick fix, Kylie!