deepcharles / ruptures

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

refactor: improve speed of BottomUp #309

Closed probberechts closed 10 months ago

probberechts commented 11 months ago

This PR makes some small modifications to improve the speed of the BottomUp segmentation algorithm, especially when applied to long time series.


Below is a simple benchmark:

import time
import ruptures as rpt

n_samples, n_dims, sigma = 100_000, 1, 2
n_bkps = 100  # number of breakpoints
signal, bkps = rpt.pw_constant(n_samples, n_dims, n_bkps, noise_std=sigma)

t1 = time.perf_counter(), time.process_time()
algo = rpt.BottomUp(model="normal").fit(signal)
my_bkps = algo.predict(n_bkps=100)
t2 = time.perf_counter(), time.process_time()
print(f" Real time: {t2[0] - t1[0]:.2f} seconds")
print(f" CPU time: {t2[1] - t1[1]:.2f} seconds")

Current implementation: 46.08 seconds
Adapted implementation: 6.94 seconds

oboulant commented 10 months ago

@probberechts Thx for the great work ! Can you push some fake changes : indeed it is strange, your created PR did not trigger the usual automated checks (tests and everything).

oboulant commented 10 months ago

This is an example of a recent PR for which I could allow the usual checks to run for a first time contributor : https://github.com/deepcharles/ruptures/pull/315

probberechts commented 10 months ago

I don't know why it didn't work, but I think you should be able to run the workflows now.

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (03ff932) 98.77% compared to head (0347762) 98.77%. Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #309 +/- ## ======================================= Coverage 98.77% 98.77% ======================================= Files 40 40 Lines 978 980 +2 ======================================= + Hits 966 968 +2 Misses 12 12 ``` | [Flag](https://app.codecov.io/gh/deepcharles/ruptures/pull/309/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Charles+T.) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/deepcharles/ruptures/pull/309/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Charles+T.) | `98.77% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Charles+T.#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

oboulant commented 10 months ago

On my end : Current implementation: 17.23 seconds Adapted implementation: 1.36 seconds

🚀