Add partial ratio

[ljnsn]

Adds

• ScoreAlignment struct
• partial_ratio
• partial_ratio_alignment
• partial_ratio_impl (short needle)

Plus tests.

[maxbachmann]

Thanks for the addition.

I made a preliminary review with some things I noticed. I believe this should cover most of it, but this will certainly require another look.

The Args interface requires a bit of design work to properly work for partial_ratio_alignment. I believe I have a solution for this, but I will need to test around with this tomorrow.

[maxbachmann]

If the two strings are swapped for the comparison we have to swap the start and end positions to fix them up. I guess that would come down to something along the lines of:

if len1 > len2 {
    let mut res = partial_ratio_alignment(s2, len2, s1, len1, args);
    std::mem::swap(&mut res.src_start, &mut res.dest_start);
    std::mem::swap(&mut res.src_end, &mut res.dest_end);
    return res;
}

Alternatively you can construct a new ScoreAlignment and return that.

A test would make sense for this as well. I should add one to the C++ tests too.

[ljnsn]

done

[maxbachmann]

This will not return None if a score below score_cutoff was passed.
For now this could be:

let score =  match alignment {
      Some(alignment) => alignment.score,
      None => 0.0,
  }
args.score_cutoff.score(score)

This will likely need to be changed when making the return type of partial_ratio_alignment alignment dependent on the presence of a score_cutoff.

Please add a test for this as well. fn issue206 might be a good place. For the other languages I managed to find ways to iterate over functions to reduce the boilerplate for some of these tests. I didn't look into ways to achieve the same in rust so far.

[ljnsn]

done

[maxbachmann]

things like the batchcomparator and the charset should be passed as argument. This is required so the same implementation can be used from PartialRatioBatchComparator. PartialRatioBatchComparator should get implemented.

[ljnsn]

done

[maxbachmann]

I am a bit unsure about this so far. In C++ we directly use random access, but that can only be done because we assume fast random access. In Rust e.g. string is utf8 and so has no fast random access.
I believe we could write this so random access isn't required. However we would still iterate the string multiple times and so maybe for those types it's still faster to just make the copy.

I would leave it like this for now, since partial_ratio is never super fast anyway. If we were to change it we should probably benchmark it first.

[maxbachmann]

I think there are still a couple of other test in https://github.com/rapidfuzz/RapidFuzz/blob/main/tests/test_fuzz.py and https://github.com/rapidfuzz/rapidfuzz-cpp/blob/main/test/tests-fuzz.cpp that we could port over, but I didn't check this in depth so far.

[ljnsn]

I implemented a couple of them a year ago, could you give this another look and let me know if there are additional ones that should be ported over?

[ljnsn]

Thanks for the comments! I've already taken a stab at improving typing and did the simplifications you suggested. Tests still to be extended.

[maxbachmann]

It assumes len(s1) <= len(s2) not len(s1) < len(s2). I fixed the comment in the Python version as well.

The implementation should be based on the C++ implementation you can fine here: https://github.com/rapidfuzz/rapidfuzz-cpp/blob/c6a3ac87c42ddf52f502dc3ed7001c8c2cefb900/rapidfuzz/fuzz_impl.hpp#L68

The Python implementation only skips windows if they can't exist since a character is unique. The C++ implementation makes use of the fact that a score can only change by a certain distance per shift of the window to skip windows. An example would be:

"aaa" <-> "bcdef"

Here the alignments with the full length are:

• "aaa" <-> "bcd"
• "aaa" <-> "cde"
• "aaa" <-> "def"

Now if we first calculate the distance for the alignments 1 and 3 both of the have a indel distance of 6. That way we know that alignment 2 can at best have an indel distance of 4.

Looking at the C++ implementation I feel like the actual min_score calculation in there is incorrect.. I believe instead of:

/* half of the cells that are not needed for known_edits can lead to a better score */
ptrdiff_t min_score =
    static_cast<ptrdiff_t>(std::min(scores[window.first], scores[window.second])) -
    static_cast<ptrdiff_t>(cell_diff + known_edits / 2);

this should be

/* half of the cells that are not needed for known_edits can lead to a better score */
size_t  max_score_improvement  = (cell_diff - known_edits / 2) / 2 * 2;
ptrdiff_t min_score =
    static_cast<ptrdiff_t>(std::min(scores[window.first], scores[window.second])) -
    static_cast<ptrdiff_t>(max_score_improvement);

The current implementation doesn't lead to incorrect results but allows skipping less cells than we really could.

[maxbachmann]

I improved the calculation of min_score in rapidfuzz-cpp to skip more alignments

[ljnsn]

done ✔️

[ljnsn]

@maxbachmann took a while, but I think all points were addressed.