Skip to content

Keep score_cutoff of normalized scorers in double precision - #495

Open
raffaelemancuso wants to merge 2 commits into
rapidfuzz:mainfrom
raffaelemancuso:fix_score_cutoff_precision
Open

raffaelemancuso wants to merge 2 commits into
rapidfuzz:mainfrom
raffaelemancuso:fix_score_cutoff_precision

Conversation

@raffaelemancuso

Copy link
Copy Markdown

Fixes #494.

get_score_cutoff_f64 in src/rapidfuzz/cpp_common.pxd stored the cutoff in a Cython float, which is a single-precision C float, although it returns a double and every caller passes a double on to rapidfuzz-cpp. A cutoff whose float32 value rounds up (0.8 becomes 0.800000011920929, likewise 0.925 and 4/7) then rejected a score exactly equal to it:

from rapidfuzz.distance import Indel
Indel.normalized_similarity(list("abcde"), list("abcdZ"), score_cutoff=0.8)
# 0.0 before, 0.8 after (and 0.8 in the pure Python implementation)

The first commit declares c_score_cutoff and the worst_score / optimal_score parameters as double.

The regression test, test_score_cutoff_equal_to_score in tests/distance/test_distance.py, checks that every normalized scorer returns a score equal to score_cutoff rather than 0 or 1, for both normalized_similarity and normalized_distance, with the C++ and the pure Python implementations alike (through the existing GenericScorer). It also caught a second, smaller divergence, fixed in the second commit: the pure Python Prefix and Postfix computed normalized_distance as 1.0 - normalized_similarity, which can differ in the last bit from distance / maximum, the value the C++ implementation returns (0.19999999999999996 instead of 0.2); with that value as score_cutoff the two implementations disagreed. They now compute it as the C++ implementation does (and as Levenshtein_py already did).

Tests

  • Against the released 3.14.6 wheel, the new test fails for 9 of the 10 scorers (Postfix scores 0 on these strings).
  • Built from this branch on Windows 11 x64 (MSVC 19.51, CPython 3.14.5): 412 passed, 19 skipped (numpy not installed).
  • CI on my fork: the test suite passes (431 passed) in "Coverage of Test Build", whose final Codecov upload fails there for lack of a token; "Submodule Test" fails as it does on main (submodule v3.9.0 against v4.1.0).

I also added two entries under an [Unreleased] heading in CHANGELOG.rst; happy to drop them if you prefer to write the changelog at release time.

get_score_cutoff_f64 stored the cutoff in a Cython float, a single-precision
C float. A cutoff that rounds up in float32 (0.8 -> 0.800000011920929)
rejected a score equal to it, so normalized_similarity(..., score_cutoff=0.8)
returned 0.0 for a similarity of exactly 0.8, unlike the pure Python
implementation.

Fixes rapidfuzz#494
The pure Python implementations computed it as 1.0 - normalized_similarity,
which can differ in the last bit from distance / maximum, the value the C++
implementation returns (0.19999999999999996 instead of 0.2). With that
value as score_cutoff, the two implementations disagreed; found by
test_score_cutoff_equal_to_score.

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.

normalized_similarity rejects a score equal to score_cutoff (cutoff rounded to float32 in get_score_cutoff_f64)

1 participant