deepcharles / ruptures

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

fix: CostMl fitting behaviour #209

Closed deepcharles closed 2 years ago

deepcharles commented 2 years ago

When called twice CostMl.fit does not recompute the CostMl.metric matrix. For instance the following code fails.

import ruptures as rpt

c = rpt.costs.CostMl()

signal_1D, _ = rpt.pw_constant(n_features=1)
signal_5D, _ = rpt.pw_constant(n_features=5)

c.fit(signal_1D)
c.fit(signal_5D)
# >> ValueError

This is because the second call of .fit() does not recompute the metric matrix as it should. Array dimensions become incoherent.

Expected behaviour. On the second call of .fit(), the Mahalanobis metric matrix should be recomputed and no error should be raised. However if the user provided a metric matrix, it should be used in priority and no new computation is needed.

codecov[bot] commented 2 years ago

Codecov Report

Merging #209 (ffed2fc) into master (e7bb659) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #209   +/-   ##
=======================================
  Coverage   96.70%   96.71%           
=======================================
  Files          40       40           
  Lines         972      973    +1     
=======================================
+ Hits          940      941    +1     
  Misses         32       32           
Impacted Files Coverage Δ
src/ruptures/costs/costml.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e7bb659...ffed2fc. Read the comment docs.