HenrikBengtsson / matrixStats

R package: Methods that Apply to Rows and Columns of Matrices (and to Vectors)
https://cran.r-project.org/package=matrixStats
203 stars 33 forks source link

BUG: colCummaxs & rowCummaxs not returning max values #101

Closed APavlides closed 7 years ago

APavlides commented 7 years ago

The tests below show the bug. Instead of returning either a matrix of min's or max's for each column or row, the function returns the input matrix.

 # colCummaxs
 x <- matrix(1:12, nrow=4, ncol=3)
 xOut <- matrixStats::colCummaxs(x)
 expectMat <- matrix(c(4,4,4,4,8,8,8,8,12,12,12,12), ncol = 3)
 testthat::expect_equal(xOut, expectMat)

 # rowCummaxs
 x <- matrix(1:12, nrow=4, ncol=3)
 xOut <- matrixStats::rowCummaxs(x)
 expectMat <- matrix(c(9,10,11,12,9,10,11,12,9,10,11,12), ncol = 3)
 testthat::expect_equal(xOut, expectMat)
mayer79 commented 7 years ago

IMHO it exactly provides what it should. Namely the cumulative maximum per column.

x <- matrix(12:1, nrow=4, ncol=3)
xOut <- matrixStats::colCummaxs(x)

will provide an example where the ouput is not the input.

PeteHaitch commented 7 years ago

@APavlides I think you're looking for is the matrixStats equivalent of pmax(x, rep(colMaxs(x), each = nrow(x))) and pmax(x, rep(rowMaxs(x), times = ncol(x))). AFAIK this doesn't yet exist but could be implemented, perhaps called colPMaxs() and rowPMaxs().

APavlides commented 7 years ago

You're right, I misunderstood the output. Thanks for clarification. This is now the test:

x = matrix(c(3, 5, 2, 1, 6, 3, 7, 8, 1), ncol = 3)
xOut <- matrixStats::colCummaxs(x)
expectMat <-  matrix(c(3, 5, 5, 1, 6, 6, 7, 8, 8), ncol = 3)
expect_equal(xOut, expectMat)
HenrikBengtsson commented 7 years ago

Please note the rich set of package tests in https://github.com/HenrikBengtsson/matrixStats/tree/master/tests/. For colCummaxs() there are: