Migrate editdistance metrics to rapidfuzz with empty-reference guard#3114
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the editdistance dependency with rapidfuzz in funasr/metrics/common.py and the setup configuration. It also handles division-by-zero scenarios in CER and WER calculations when the reference length is zero. A new test file is introduced to verify these changes. The reviewer recommends simplifying the test file by removing the complex mocking of rapidfuzz and dynamic module loading, and instead using standard imports and the real library.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import importlib.util | ||
| import sys | ||
| import types | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| def _install_fake_rapidfuzz(): | ||
| rapidfuzz = types.ModuleType("rapidfuzz") | ||
| distance = types.ModuleType("rapidfuzz.distance") | ||
|
|
||
| class Levenshtein: | ||
| @staticmethod | ||
| def distance(left, right): | ||
| left = list(left) | ||
| right = list(right) | ||
| dp = list(range(len(right) + 1)) | ||
| for i, lval in enumerate(left, 1): | ||
| prev = dp[0] | ||
| dp[0] = i | ||
| for j, rval in enumerate(right, 1): | ||
| old = dp[j] | ||
| dp[j] = min( | ||
| dp[j] + 1, | ||
| dp[j - 1] + 1, | ||
| prev + (0 if lval == rval else 1), | ||
| ) | ||
| prev = old | ||
| return dp[-1] | ||
|
|
||
| distance.Levenshtein = Levenshtein | ||
| rapidfuzz.distance = distance | ||
| sys.modules.setdefault("rapidfuzz", rapidfuzz) | ||
| sys.modules.setdefault("rapidfuzz.distance", distance) | ||
|
|
||
|
|
||
| def _load_metrics_common(): | ||
| _install_fake_rapidfuzz() | ||
| module_path = Path(__file__).resolve().parents[1] / "funasr" / "metrics" / "common.py" | ||
| spec = importlib.util.spec_from_file_location("metrics_common_probe", module_path) | ||
| module = importlib.util.module_from_spec(spec) | ||
| sys.modules["metrics_common_probe"] = module | ||
| assert spec.loader is not None | ||
| spec.loader.exec_module(module) | ||
| return module | ||
|
|
||
|
|
||
| def test_cer_and_wer_return_none_when_reference_length_is_zero(): | ||
| common = _load_metrics_common() | ||
| calc = common.ErrorCalculator( | ||
| char_list=["<blank>", "a", "b", "c", "h", "e", "l", "o", "w", "r", "d", " "], | ||
| sym_space=" ", | ||
| sym_blank="<blank>", | ||
| report_cer=True, | ||
| report_wer=True, | ||
| ) | ||
|
|
||
| assert calc.calculate_cer(["abc"], [" "]) is None | ||
| assert calc.calculate_wer(["hello world"], [" "]) is None | ||
|
|
||
|
|
||
| def test_cer_and_wer_keep_normal_levenshtein_semantics(): | ||
| common = _load_metrics_common() | ||
| calc = common.ErrorCalculator( | ||
| char_list=["<blank>", "a", "b", "c", "x", "y", " "], | ||
| sym_space=" ", | ||
| sym_blank="<blank>", | ||
| report_cer=True, | ||
| report_wer=True, | ||
| ) | ||
|
|
||
| assert calc.calculate_cer(["abc"], ["axc"]) == 1 / 3 | ||
| assert calc.calculate_wer(["foo bar baz"], ["foo qux baz"]) == 1 / 3 |
There was a problem hiding this comment.
Mocking rapidfuzz with a custom pure-Python Levenshtein implementation and using dynamic module loading (importlib.util) is an anti-pattern here. Since rapidfuzz is a required dependency in setup.py, it is guaranteed to be installed in any standard test environment. Mocking it defeats the purpose of testing the actual integration with the library, hides potential import/compatibility issues, and adds unnecessary complexity to the test suite. Simplifying the test file to use standard imports and the real rapidfuzz library makes the tests cleaner, more maintainable, and more reliable.
from funasr.metrics.common import ErrorCalculator
def test_cer_and_wer_return_none_when_reference_length_is_zero():
calc = ErrorCalculator(
char_list=["<blank>", "a", "b", "c", "h", "e", "l", "o", "w", "r", "d", " "],
sym_space=" ",
sym_blank="<blank>",
report_cer=True,
report_wer=True,
)
assert calc.calculate_cer(["abc"], [" "]) is None
assert calc.calculate_wer(["hello world"], [" "]) is None
def test_cer_and_wer_keep_normal_levenshtein_semantics():
calc = ErrorCalculator(
char_list=["<blank>", "a", "b", "c", "x", "y", " "],
sym_space=" ",
sym_blank="<blank>",
report_cer=True,
report_wer=True,
)
assert calc.calculate_cer(["abc"], ["axc"]) == 1 / 3
assert calc.calculate_wer(["foo bar baz"], ["foo qux baz"]) == 1 / 3
Summary
Maintainer-owned replacement for #3105. Thanks to @excosy for starting the
editdistance->rapidfuzzmigration.This PR:
editdistancewithrapidfuzz.distance.Levenshteinrapidfuzz>=3.0.0Root Cause
The original migration guarded only on whether edit-distance entries existed. That still divides by zero when the input list is non-empty but every reference is empty or whitespace-only:
This PR calculates total reference length explicitly and returns
Nonewhen it is zero, matching the existing CTC metric no-valid-reference behavior.Testing
Additional validation on ind-gpu8:
tests/test_metrics_common_rapidfuzz.pyfailed withZeroDivisionErrorrapidfuzz>=3.0.0install verified string and token-listLevenshtein.distance()semantics