Repository navigation
Compute lcs-length and edit-distance with Myers' O(ND) algorithm - #14
Conversation
lcs-length summed the Eq hunks of diff, but diff matches the longest common contiguous block first and recurses on both sides, which does not find a longest common subsequence: for [1 2 3] and [3 1 3] it returned 1 instead of 2, so edit-distance said 4 instead of 2 and similarity 0.333 instead of 0.667. The length D of the shortest edit script (insertions plus deletions) now comes from the greedy forward pass of E. Myers, "An O(ND) Difference Algorithm and Its Variations" (1986), sections 2-3: one array of furthest-reaching x per diagonal, no traceback, elements compared with = only. lcs-length is (N+M-D)/2 and edit-distance is D; similarity is unchanged on top of lcs-length. Each round only visits the diagonals a path with at most N rightward and M downward moves can end on, which yields the same D and keeps short-vs-long inputs from costing O(D^2). diff and everything built on it are unchanged.
There was a problem hiding this comment.
Build & Tests
1c0af295builds, andcarp -x tests/diff.carppasses 95/0 locally (armhf). Master gives 77/0. CI is green on ubuntu-latest and macos-latest.- I ran the branch's test file against master's
diff.carp: 88/7. The 7 failures are exactly the new out-of-order, repeated and interleaved assertions, and master returns the values in the body's table (1/4, 2/3, 2/8, 0.25). tests/diff.carpis +74/−0. It only adds assertions; no existing one is moved or changed.- Local angler and carp-fmt report both changed files clean. CI runs both and passed.
carp --log-memory: the balance is 0 afterlcs-length,edit-distanceandsimilarityon Int, String, Char and empty arrays.
Findings
I found no correctness problems. Here is what I ran.
Differential against a Carp DP LCS (not committed). It checks lcs-length against the DP, edit-distance against N+M−2·LCS, and similarity against 2·LCS/(N+M), or 1.0 for two empty arrays.
| input set | pairs | branch mismatches | master mismatches |
|---|---|---|---|
| exhaustive, alphabet 3, lengths 0–5 per side | 132,496 | 0 | 9,618 |
| exhaustive, alphabet 2, lengths 0–7 per side | 65,025 | 0 | 10,044 |
| random Int: N ≤ 200 vs M ≤ 10, M ≤ 200 vs N ≤ 10, balanced ≤ 120, and an edited copy; alphabets 2–26 | 20,000 | 0 | 6,589 |
| random String and Char arrays | 8,000 | 0 | 2,772 |
On master the three measures fail together: every wrong LCS also gives a wrong edit distance and similarity. The first master mismatch the harness printed was [1 0] / [0 1 2 0], with lcs 1 instead of 2.
A second, separately written harness agrees: 6,000 random pairs (balanced up to 39 each, 149 vs 7, and 7 vs 149; alphabets of 1–5 symbols) give 0 mismatches on the branch and 1,103 on master, for all three functions.
Mutation battery. I applied one mutant at a time to diff.carp. Each ran the suite and the differential; the differential skipped the alphabet-2 set and checked 1,000 String/Char iterations. The differential column gives LCS mismatches for the three sets in order: alphabet 3 of 132,496, random Int of 20,000, String/Char of 2,000.
| mutant | suite | differential |
|---|---|---|
< → <= in the furthest-reaching choice (diff.carp:131) |
93/2, exactly the two out-of-order tests | 19,326 / 4,315 / 542 |
drop the k = -d case (:129) |
88/7 | 48,390 / 2,627 / 362 |
termination and → or (:141) |
82/13 | 81,660 / 15,008 / 1,004 |
termination without the y check (:141) |
85/10 | 58,146 / 8,975 / 716 |
lower clamp tightened by one diagonal (:128) |
hangs | hangs |
upper clamp tightened by one diagonal (:128) |
hangs | hangs |
unrestricted sweep, k in [-d, d] |
95/0 | 0 / 0 / 0 |
| both clamps loosened by one diagonal | 95/0 | 0 / 0 / 0 |
off one smaller (:122) |
aborts on unsafe-nth's n < a.len assert |
aborts |
d starting at 0 (:124) |
hangs | hangs |
snake without the (- x k) < m guard (:136) |
aborts on the same assert | aborts |
drop (/= k d) (:130) |
95/0 | 0 / 0 / 0 |
The "hangs" rows ran at about 90% CPU for 45–80 s before I killed them. The unmutated suite binary exits in 0.01 s.
The last row is an equivalent mutant, not a test gap. Diagonal d+1 is never written before round d+1, so its slot is still 0, and every x is ≥ 0. At k = d the comparison V[k-1] < V[k+1] is therefore false with or without the guard.
The unrestricted and loosened rows confirm, over 154k pairs, the body's claim that the clamped sweep finds the same D as the unrestricted one. Every mutant result the body reports reproduces.
Cost. Default build, one run each. Load was 2.5–2.7 because other jobs were running.
| shape | master | this PR |
|---|---|---|
| identical, 5000 each | 21.3 ms | 0.65 ms |
| disjoint, 5000 each | 15.1 ms | 4940 ms |
| disjoint, 5000 vs 50 | 7.6 ms | 50.6 ms |
| random, 2000 each, alphabet 4 | 2442 ms (lcs 205, wrong) | 218 ms (lcs 1289) |
| 5000 vs the same with 50 replaced | 535 ms | 1.6 ms |
[1 2 3] vs 5000 |
15.1 ms | 3.6 ms |
The body's numbers reproduce. My random arrays are different ones, so that row's lcs values differ from the body's. I added the skewed near-disjoint row: it is about 7× slower than master, as O(min(N,M)·D) predicts. That is the same cost as the disjoint row, just smaller.
So the price of this PR is that lcs-length, edit-distance and similarity are quadratic on two large, mostly unrelated inputs, where master returned at once. Nothing else in the org is affected:
- none of the other 46 local clones loads this package or calls
Diff.; - a GitHub code search of the org finds
carpentry-org/diffonly in the website's library list.
Whether correct answers are worth that worst case is your call, and the body already says so.
I also checked the claim that the elements no longer need to be hashable. A type with only = compiles on the branch and gives lcs 1 / edit 1. Master rejects it because there is no hash.
Verdict: merge
All three functions now return the true LCS-based values, with 0 mismatches over 225,521 pairs against 29,023 on master. Every mutant claim in the body reproduces. The only open question is the documented slowdown on unrelated inputs, and that trade-off is yours to make.
Diff.lcs-length,Diff.edit-distanceandDiff.similarityare documented as longest-common-subsequence measures, butlcs-lengthsummed theEqhunks ofDiff.diff.diffis simplediff: it matches the longest common contiguous block, then recurses on both sides. That does not find an LCS, so whenever the greedy block choice misses one,lcs-lengthis too low,edit-distancetoo high andsimilaritytoo low.[1 2 3]/[3 1 3][1 1 1]/[1 2 1 1][3 1 2 1 3 2]/[3 2 1 2 3 2][1 2]/[2 1 1 1 3 2]Change
A private
ses-lengthcomputes the length D of the shortest edit script (insertions plus deletions) with the greedy forward pass of E. Myers, An O(ND) Difference Algorithm and Its Variations (1986), sections 2–3. It keeps one array of furthest-reaching x per diagonal, does no traceback and compares elements with=only.lcs-lengthis(N+M-D)/2,edit-distanceisD, andsimilarityis unchanged on top oflcs-length(two empty arrays are still 1.0). Because only=is needed now, these three functions no longer require the elements to be hashable.Each round visits only diagonals
kin[max(-D, D-2M), min(D, 2N-D)], the ones a D-path with at most N rightward and M downward moves can end on. The neighbours those diagonals read always come from the previous round's range, so this yields the same D as the unrestricted sweep (checked exhaustively below). It halves the sweep for equal-length inputs and turns short-vs-long inputs from O(D²) into O(min(N,M)·D) diagonal steps.Complexity: O((N+M)·D) time and O(N+M) space, against master's O(N·M) with Map allocations per recursion level of
diff.Diff.diffand everything built on it (unified,patch,apply,revert) are untouched.Tests
tests/diff.carpkeeps every existing assertion and adds:Expected values come from a Python DP LCS. Similarity is asserted only where the value is exact in binary (0.5, 0.75).
Not committed, run locally:
Diff.lcs-lengthagainst a Carp DP LCS on 23,000 random Int arrays (lengths 0–119), plus String and Char arrays: 0 mismatches. On master the same harness reports 4,186 mismatches.<→<=in the furthest-reaching choice fails only the two new out-of-order tests. Dropping thek = -Dcase or weakening the termination check fails 7 and 10 tests, existing and new. Tightening either clamp bound by one diagonal hangs the suite.Cost
lcs-length, single run on a Raspberry Pi 500 (Cortex-A76, armhf). The default build is whatcarp -xgives you; the second table is the same benchmark built with--optimize.[1 2 3]vs 5000 elementsWith
--optimize:[1 2 3]vs 5000 elementsThe regression is inputs with almost nothing in common, which is O(ND)'s worst case. There D = N+M, so the sweep is quadratic (25M diagonal steps for 5000 vs 5000). On master,
difffinds no common block, so it returns almost immediately with lcs 0. Every input with real overlap gets faster, and master's random-alphabet result was wrong anyway.Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.