Repository navigation
Keep score_cutoff of normalized scorers in double precision - #495
Open
raffaelemancuso wants to merge 2 commits into
Open
raffaelemancuso wants to merge 2 commits into
raffaelemancuso wants to merge 2 commits into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #494.
get_score_cutoff_f64insrc/rapidfuzz/cpp_common.pxdstored the cutoff in a Cythonfloat, which is a single-precision C float, although it returns adoubleand every caller passes adoubleon 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:The first commit declares
c_score_cutoffand theworst_score/optimal_scoreparameters asdouble.The regression test,
test_score_cutoff_equal_to_scoreintests/distance/test_distance.py, checks that every normalized scorer returns a score equal toscore_cutoffrather than 0 or 1, for bothnormalized_similarityandnormalized_distance, with the C++ and the pure Python implementations alike (through the existingGenericScorer). It also caught a second, smaller divergence, fixed in the second commit: the pure PythonPrefixandPostfixcomputednormalized_distanceas1.0 - normalized_similarity, which can differ in the last bit fromdistance / maximum, the value the C++ implementation returns (0.19999999999999996 instead of 0.2); with that value asscore_cutoffthe two implementations disagreed. They now compute it as the C++ implementation does (and asLevenshtein_pyalready did).Tests
Postfixscores 0 on these strings).main(submodule v3.9.0 against v4.1.0).I also added two entries under an
[Unreleased]heading inCHANGELOG.rst; happy to drop them if you prefer to write the changelog at release time.