Skip to content

Fix ctw_path never iterating because the path was compared with itself (#728) - #730

Open
cpruijsen wants to merge 1 commit into
tslearn-team:mainfrom
cpruijsen:fix/issue-728
Open

Fix ctw_path never iterating because the path was compared with itself (#728)#730
cpruijsen wants to merge 1 commit into
tslearn-team:mainfrom
cpruijsen:fix/issue-728

Conversation

@cpruijsen

Copy link
Copy Markdown

Fixes #728.

Summary

  • ctw_path compared the current alignment path with itself, so the loop always stopped after the first DTW. The returned score was the identity-projection DTW; max_iter was a no-op; the path and the score disagreed; verbose only printed iteration 0.
  • The loop now compares against the previous path and always keeps the path and score from the same dtw_path call.
  • sklearn's CCA does not enforce the paper's unit-covariance constraint on the two views. After each fit the canonical axes are rescaled so each warped view has unit standard deviation before DTW. That is also what stops the period-2 path cycle on the series from [WIP] ctw fixes #579 (the paper's alternation is monotonically decreasing; Zhou's MATLAB code stops on path stability, not cycle detection).

What I chose, and the alternative. Path-stability stop plus unit-covariance rescaling of the existing sklearn object. Alternative: typo-only, as in #579, that iterates and fixes the four bullets above, but on PR 579's series the path cycles and ctw(..., max_iter=2) != ctw(..., max_iter=3). Happy to drop the rescaling if you want the smaller diff, or to replace sklearn CCA with the paper's generalized eigenproblem if you want a stricter match.

This comparison has been in ctw.py since CTW landed. Callers that treated metric="ctw" as DTW (including the k-neighbors unit test) will see different rankings; that is the bugfix.

Test plan

  • ctw(..., max_iter=1) still equals DTW; default ctw does not, on the issue's rotated 3-D series
  • returned path and score match dtw_path(*cca.transform(s1, s2))
  • max_iter=5 equals max_iter=100 on that series (path has stabilized)
  • PR 579 series: max_iter=2 equals max_iter=3 (no parity-dependent score)
  • existing 1-D doctests: linearly related series still have distance 0; imperfect 1-D pair is the canonical-space DTW, not identity DTW
  • KNeighborsTimeSeries(metric="ctw") neighbor order is no longer identical to DTW

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.99%. Comparing base (3f2daeb) to head (e78273e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #730      +/-   ##
==========================================
+ Coverage   95.96%   95.99%   +0.03%     
==========================================
  Files          87       87              
  Lines        9259     9288      +29     
==========================================
+ Hits         8885     8916      +31     
+ Misses        374      372       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ctw_path never iterates: convergence check compares current_path with itself

1 participant