bfast2 / bfast

Breaks For Additive Season and Trend
https://bfast2.github.io
GNU General Public License v2.0
41 stars 15 forks source link

bfastts inconsistency with .bfastts.new #75

Closed GreatEmerald closed 3 years ago

GreatEmerald commented 3 years ago

When the input to bfastts is a matrix, the new implementation only takes the first variable. This should be fixed before we can enable the new method by default.

Reprex:

library(bfast)
library(strucchange)
#> Loading required package: zoo
#> 
#> Attaching package: 'zoo'
#> The following objects are masked from 'package:base':
#> 
#>     as.Date, as.Date.numeric
#> Loading required package: sandwich
library(raster)
#> Loading required package: sp

modisraster <- brick(system.file("extdata/modisraster.grd", package="bfast"))
#> Warning in showSRID(uprojargs, format = "PROJ", multiline = "NO", prefer_proj
#> = prefer_proj): Discarded datum Unknown based on Clarke 1866 ellipsoid in CRS
#> definition
set_default_options()
bfts1 = bfastts(modisraster[1], dates, "16-day")
set_fast_options()
bfts2 = bfastts(modisraster[1], dates, "16-day")
class(bfts1)
#> [1] "mts"    "ts"     "matrix"
class(bfts2)
#> [1] "ts"

Created on 2021-03-18 by the reprex package (v1.0.0)

appelmar commented 3 years ago

Thanks, .bfastts.new indeed does not handle matrices and I'll work on this. However, I am not sure what the desired output of the example is. I get

dim(modisraster[1])
#> [1]   1 275
dim(bfts1)
#> [1] 275 275

It seems that bfts1 has 275 times the same time series. Is this expected?

appelmar commented 3 years ago

.bfastts.new() should now work for multiple time series in a matrix (rows = time, cols = variables). However, the raster example with modisraster[1] contains a single time series in a matrix with 275 rows and hence does not produce a meaningful output (same for the original bfastts implementation).