Problem
HungarianMatcher.calculate_metrics derives fn and fp as len(list) - tp, so a pair that appears in matched_pairs but scored below match_threshold is counted in both places at once:
m = HungarianMatcher(comparator=LevenshteinComparator(), match_threshold=0.9)
m.calculate_metrics(["abc", "def"], ["abd", "deg"])
# matched_pairs = [(0,0,0.667), (1,1,0.667)] -> every item has a partner
# fn = 2 -> ...and yet 2 false negatives
This contradicts the documented model, where only items with no partner become FN or FA (Hungarian matching). HungarianMatcher is public API, exported from stickler.algorithms, so a caller reading fn as "GT items with no partner" gets the wrong answer at every list length.
Why it is not already fixed
StructuredListComparator reads matched_pairs and reclassifies, so no user-visible confusion-matrix metric is wrong today. The five-category split (TP/TN/FN/FA/FD) exists downstream; calculate_metrics predates it and only has the three-way vocabulary.
Documented as a Note on the method in #225 rather than fixed, because changing the return shape is not in scope for a fast-path bug fix.
Proposed direction
Give calculate_metrics an explicit fd key and make fn/fp mean what the docs say:
fd: matched pairs scoring below match_threshold
fn: GT items with no partner
fp: fa + fd, preserving the existing identity
ComparisonHelper.unordered_list_metrics already produces exactly this shape, so the vocabulary and precedent exist.
Compatibility
Breaking for anything reading fn/fp off calculate_metrics directly. In-repo callers would need auditing; tests/common/algorithms/test_hungarian.py asserts these keys heavily.
Note tests/common/algorithms/test_hungarian_path_parity.py::test_zero_similarity_counts_are_length_independent pins today's fp == n / fn == n and carries a comment saying those values are descriptive rather than contractual. It will need updating alongside.
Acceptance criteria
Notes
Found by @vawsgit reviewing #225. Deferred from that PR deliberately.
Problem
HungarianMatcher.calculate_metricsderivesfnandfpaslen(list) - tp, so a pair that appears inmatched_pairsbut scored belowmatch_thresholdis counted in both places at once:This contradicts the documented model, where only items with no partner become FN or FA (Hungarian matching).
HungarianMatcheris public API, exported fromstickler.algorithms, so a caller readingfnas "GT items with no partner" gets the wrong answer at every list length.Why it is not already fixed
StructuredListComparatorreadsmatched_pairsand reclassifies, so no user-visible confusion-matrix metric is wrong today. The five-category split (TP/TN/FN/FA/FD) exists downstream;calculate_metricspredates it and only has the three-way vocabulary.Documented as a
Noteon the method in #225 rather than fixed, because changing the return shape is not in scope for a fast-path bug fix.Proposed direction
Give
calculate_metricsan explicitfdkey and makefn/fpmean what the docs say:fd: matched pairs scoring belowmatch_thresholdfn: GT items with no partnerfp:fa + fd, preserving the existing identityComparisonHelper.unordered_list_metricsalready produces exactly this shape, so the vocabulary and precedent exist.Compatibility
Breaking for anything reading
fn/fpoffcalculate_metricsdirectly. In-repo callers would need auditing;tests/common/algorithms/test_hungarian.pyasserts these keys heavily.Note
tests/common/algorithms/test_hungarian_path_parity.py::test_zero_similarity_counts_are_length_independentpins today'sfp == n/fn == nand carries a comment saying those values are descriptive rather than contractual. It will need updating alongside.Acceptance criteria
calculate_metricsreturnsfd, andfn/fpfollow the documented partnering semanticsNoteadded in fix(hungarian): align the single-item fast path with the general path (#224) #225 is replaced by the real contractHungarianMatchercallersNotes
Found by @vawsgit reviewing #225. Deferred from that PR deliberately.