search

rkillick / changepoint

A place for the development version of the changepoint package on CRAN.
127 stars 33 forks source link

confusing errors with Q too large in binseg #46

Closed tdhock closed 2 years ago

tdhock commented 3 years ago

hi again @rkillick, after pulling current master that includes your changes from #45 , I now see

> changepoint::cpt.mean(c(1,2,4), penalty="Manual", method="BinSeg", pen.value=0, Q=4)@param.est
Error in BINSEG(sumstat, pen = pen.value, cost_func = costfunc, minseglen = minseglen,  : 
  Q is larger than the length of the data length
> changepoint::cpt.mean(c(1,2,4), penalty="Manual", method="BinSeg", pen.value=0, Q=5)@param.est
Error in BINSEG(sumstat, pen = pen.value, cost_func = costfunc, minseglen = minseglen,  : 
  Q is larger than the maximum number of segments 4
>

I was expecting these two errors to be the same. And I would have expected it to say the max number of segments it 3 (because there are 3 data points). Also the error "Q is larger than the max number of segments" may be confusing because Q is documented as "The maximum number of changepoints"

rkillick commented 2 years ago

I'm not sure what caused this but it looks like it is fixed in the latest version 2.2.3 just sent to CRAN.

tdhock commented 2 years ago

Hi @rkillick thanks for the update! Now I see 

> changepoint::cpt.mean(c(1,2,4), penalty="Manual", method="BinSeg", pen.value=0, Q=4)@param.est
Error in BINSEG(sumstat, pen = pen.value, cost_func = costfunc, minseglen = minseglen,  : 
  Q is larger than the maximum number of segments 2.5
> changepoint::cpt.mean(c(1,2,4), penalty="Manual", method="BinSeg", pen.value=0, Q=5)@param.est
Error in BINSEG(sumstat, pen = pen.value, cost_func = costfunc, minseglen = minseglen,  : 
  Q is larger than the maximum number of segments 2.5

This output is definitely better since the error messages are consistent. However I am confused about the new error: what does it mean to have 2.5 segments? Also my previous suggestion to change "segments" to "changepoints" would still be useful. As it is the error gives me impression that Q is the number of segments, but it is actually the number of changepoints, right?

tdhock commented 2 years ago

I suggest the following error: "Q is larger than the maximum number of changepoints 2"

rkillick commented 2 years ago

I've added the floor argument to the version headed to CRAN. Sorry I hadn't committed to github prior to sending the message.