deepcharles / ruptures

ruptures: change point detection in Python
BSD 2-Clause "Simplified" License
1.59k stars 162 forks source link

Too strict sanity check for Pelt model? #188

Closed ChiQiao closed 3 years ago

ChiQiao commented 3 years ago

Sanity check was added in v1.1.4 for the Pelt class, and raises a BadSegmentationParameters for the example below

import ruptures
import numpy as np

model = ruptures.Pelt(min_size=5, jump=1)
model.fit(np.ones(9))
model.predict(pen=0)

For a signal with a length of 9, min_size=5 makes sense to me and in v1.1.3 it returns [9]. I wonder if the sanity check is too strict?

oboulant commented 3 years ago

Hi,

Thanks for your interest in ruptures !

Here, you are giving as an input

With those parameters, the sanity checks verify that it can generate at least 2 segments (ie. 1 break point). Here, what you are presently asking is not feasible, indeed :

Walkthrough in the Source Code

Alternatives for you situation

In your case, the following choices would work :

model = ruptures.Pelt(min_size=5, jump=1)
model.fit(np.ones(10))
model.predict(pen=0)
model = ruptures.Pelt(min_size=4, jump=1)
model.fit(np.ones(9))
model.predict(pen=0)
model = ruptures.Pelt(min_size=5, jump=2)
model.fit(np.ones(11))
model.predict(pen=0)
deepcharles commented 3 years ago

@ChiQiao Indeed it makes sense to return [9]. We will see how this change propagates through the rest of the package.

Thanks

oboulant commented 3 years ago

Oh, did not pay attention to the Pelt mention and the detection methods that may return 0 break point (even more with a 0 penalty). Will look into it !