Skip to content

fix(chord): tolerate tiny overlaps in directional Hamming distance - #437

Open
BenjaminTay wants to merge 1 commit into
mir-evaluation:mainfrom
BenjaminTay:fix/chord-interval-float-stability
Open

BenjaminTay wants to merge 1 commit into
mir-evaluation:mainfrom
BenjaminTay:fix/chord-interval-float-stability

Conversation

@BenjaminTay

@BenjaminTay BenjaminTay commented Oct 2, 2026 •

Copy link
Copy Markdown

Fixes #295.

The boundary discrepancy reported in the issue (~1 ns) and its float64 frame-arithmetic example (~1.11e-16 s) can make chord.directional_hamming_distance raise ValueError, preventing chord evaluation from returning scores.

This change treats adjacent intervals as touching only when their overlap is at most 10 ns and 1 ppm of the shorter neighboring duration. It moves the later start to the preceding end in a local copy. Reference and estimated intervals use the same adjustment before the adjacent-overlap check and scoring; caller arrays and all positive gaps are preserved.

Engineering tradeoffs

  • 10 ns gives a tenfold margin over the boundary discrepancy reported in the issue. 1 ppm limits duration changes and protects short intervals from collapse. These are engineering choices proposed for maintainer review, not established tolerances or machine-epsilon bounds. Unlike whole-array millisecond rounding, adjustments remain local.
  • Genuine micro-overlaps within both limits are also absorbed: the values alone cannot distinguish them from numerical artifacts.
  • The limits do not adapt to dtype or timestamp scale. The original float64 examples are covered; the analogous float32 frame example can overlap by ~5.96e-8 seconds and still raises.

Validation

The 11 added cases cover reported inputs, both scoring directions, read-only inputs, preserved gaps, and rejected material overlaps/short intervals. Against the unmodified base they give 4 failed / 7 passed; with this patch, 11 passed.

Full local runs used the same macOS/Python 3.12 environment and base fe73b3533737814f83dbd9739f06e90f5f82f758, running pytest --mpl-results-path=mpl_image_results/ from tests/ with default options:

Source Passed Failed Skipped Xfailed Xpassed
Unmodified base 516 20 3 175 1
This patch 527 20 3 175 1

The full suite did not pass. Both runs exited 1 with the same 20 test_display.py image failures: matching test IDs and RMS values, and 60 byte-identical comparison PNGs. This comparison found no additional failures; it does not replace upstream CI. Image baselines were not changed.

@BenjaminTay
BenjaminTay marked this pull request as ready for review October 2, 2026 16:53
- 修复 JAMS 序列化和 float64 分帧算术导致的相邻区间重叠误报,关联 mir-evaluation#295。
- 在 chord 度量内对参考与估计使用同一局部副本,仅修正同时满足 10 ns 和相邻较短时长 1 ppm 门限的重叠;保留所有正间隙,不修改输入数组。
- 新增 11 个回归用例:原始源码 4 failed / 7 passed,补丁 11 passed;具体门限作为维护者审议方案,尚不覆盖所有浮点精度。
- 上游默认完整测试入口得到 527 passed / 20 failed,原始同 SHA 基线为 516 passed / 20 failed;失败均为相同图像比较,RMS 与 60 个对应 PNG 一致。
- Black、flake8、pydocstyle 及 diff 检查通过。
@BenjaminTay
BenjaminTay force-pushed the fix/chord-interval-float-stability branch from 6ddb941 to fff4f68 Compare October 2, 2026 17:10
@craffel

craffel commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

@bmcfee what do you think of this approach for fixing #295 which you had originally raised?

@bmcfee

bmcfee commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

It seems pretty reasonable to me. I'll also loop in @tomxi - would this intersect with your exact segment code in a useful way?

This branch has not been deployed

No deployments
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.

Numerical stability issues in floating point comparisons (chord directional hamming)

3 participants