Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(CostL2): set min_size to 1 #255

Merged
merged 7 commits into from
May 17, 2022
Merged

fix(CostL2): set min_size to 1 #255

merged 7 commits into from
May 17, 2022

Conversation

deepcharles
Copy link
Owner

The min_size parameter was set to 2 for CostL2 and produced bad segmentations for small signals (see Issue #242).

Now it is set to 1 by default.

@deepcharles
Copy link
Owner Author

resolves #242

@github-actions github-actions bot added the Type: Fix Bug or Bug fixes label Apr 29, 2022
@codecov
Copy link

codecov bot commented Apr 29, 2022

Codecov Report

Merging #255 (fbc6e4d) into master (939e7e2) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #255   +/-   ##
=======================================
  Coverage   98.77%   98.77%           
=======================================
  Files          40       40           
  Lines         978      978           
=======================================
  Hits          966      966           
  Misses         12       12           
Flag Coverage Δ
unittests 98.77% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/ruptures/costs/costl2.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 939e7e2...fbc6e4d. Read the comment docs.

@deepcharles deepcharles requested a review from oboulant April 29, 2022 10:04
@oboulant
Copy link
Collaborator

oboulant commented May 9, 2022

I would add to the tests a special case for CostL2 where we explicitly check that the cost computed on a segment of size 1 returns 0.0 (and do not throw an NotEnoughPoints).

@deepcharles deepcharles merged commit ed9bb0c into master May 17, 2022
@deepcharles deepcharles deleted the fix/minsize-l2 branch May 17, 2022 06:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Type: Fix Bug or Bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants