Skip to content

[WIP] Add exact EMD-L1 solver for Cartesian grid histograms - #863

Open
tvercaut wants to merge 9 commits into
PythonOT:masterfrom
tvercaut:solve_grid_l1
Open

tvercaut wants to merge 9 commits into
PythonOT:masterfrom
tvercaut:solve_grid_l1

Conversation

@tvercaut

@tvercaut tvercaut commented Sep 18, 2026 •

Copy link
Copy Markdown

Summary

Follow-up to discussion #862: adds ot.lp.emd_grid_l1(A, B), an exact solver for the Earth Mover's Distance with the cityblock (L1) ground metric between two histograms sharing the same d-dimensional Cartesian grid support.

Instead of solving the min-cost flow on the full bipartite graph between the n = prod(A.shape) source and target bins (as ot.emd/ot.emd2 would), this reduces the problem to a min-cost flow on the much sparser grid adjacency graph (O(d*n) arcs instead of O(n^2)), following the graph formulation of Ling & Okada [1]. Unlike that paper, which introduces a bespoke tree-based solver for the reduced graph, this reuses POT's existing (off-the-shelf) NetworkSimplexSimple LP solver on it — as suggested in the discussion, this is a small, well-contained addition on top of infrastructure POT already ships.

[1] Ling, H., & Okada, K. (2007). An efficient earth mover's distance algorithm for robust histogram comparison. IEEE TPAMI, 29(5), 840-853.

What's in this PR

  • ot/lp/sparse_digraph.h: a new general (non-bipartite) sparse digraph. The existing sparse_bipartitegraph.h (from [MRG] Sparse emd implementation #778) splits nodes into a source half and a target half; that doesn't work here since every grid cell can carry both supply and demand at once.
  • EMD_wrap_grid_l1 in EMD_wrapper.cpp/EMD.h: builds the grid adjacency graph directly from a shape array, runs NetworkSimplexSimple on it, and (only when requested) decomposes the resulting Beckmann-style arc flow into an explicit transportation plan — including the "self-mass" A and B already share at the same bin, which the flow decomposition alone misses (more on this below).
  • ot/lp/_grid.py (emd_grid_l1): the Python-facing entry point.
    • A, B are passed as actual d-dimensional arrays (not flattened + a separate shape argument) so they naturally carry their own grid geometry.
    • return_plan=False by default: recovering the plan has a real cost of its own (network simplex flow decomposition, or an O(n) merge in 1D), so it's opt-in.
    • Dedicated 1D fast path. A 1D grid is just a shared, sorted, unit-spaced support, so:
      • the plan (when requested) is recovered via POT's own emd_1d_sorted, skipping the network-simplex setup entirely;
      • the cost-only case (the default) has a closed form — the L1 norm of the difference of cumulative sums — computed with generic backend reductions (nx.cumsum/nx.abs/nx.sum) and no CPU round-trip at all, verified end-to-end on an MPS GPU tensor.
    • Backend-compatible throughout (numpy/torch/jax/tf/cupy via ot.backend). The general (ndim >= 2) path and the 1D-with-plan path do need a CPU round-trip for the compiled solver, same as ot.emd/ot.emd2_lazy.
    • The sparse plan is returned as log["G"], a sparse matrix built via the backend's coo_matrix — the same mechanism and convention ot.emd2_lazy's return_matrix already uses (real sparse type for NumPy/PyTorch/TensorFlow/CuPy, densified for JAX, which has no sparse array type).
  • test/test_grid.py: correctness against the dense solver on random grids (1D-4D), plan/coupling marginal checks (row/col sums of G match A/B exactly, not just the net residual), backend round-trips (including dtype/device preservation), and dispatch checks (mocking confirms the 1D path never touches the general C++ solver, and the plan-less 1D path never touches the O(n) merge either).

Benchmarks

Script attached at the bottom of this description (local_sandbox/bench_grid_l1.py, not committed). Compares emd_grid_l1 against ot.emd2 (dense) and ot.emd2_lazy for ndim 2-4, and against ot.emd2_1d (POT's own dedicated 1D solver) for ndim=1, across a range of grid resolutions. All costs match exactly wherever compared (asserted in the script); a couple of the largest dense/lazy runs needed numItermax raised from the default 100000 to 2,000,000 to actually converge on the full bipartite graph at that size (noted below).

ndim bins/dim n reference solver grid time lazy time reference time lazy/grid reference/grid
1 10 10 emd2_1d 0.05 ms - 0.06 ms - 1.4x
1 100 100 emd2_1d 0.04 ms - 0.04 ms - 1.0x
1 1000 1000 emd2_1d 0.05 ms - 0.06 ms - 1.3x
1 10000 10000 emd2_1d 0.13 ms - 0.36 ms - 2.8x
1 100000 100000 emd2_1d 1.00 ms - 3.51 ms - 3.5x
2 10 100 emd2 (dense) 0.09 ms 1.14 ms 0.49 ms 13.1x 5.6x
2 20 400 emd2 (dense) 0.37 ms 19.79 ms 8.87 ms 53.4x 23.9x
2 32 1024 emd2 (dense) 1.73 ms 133.31 ms 61.72 ms 77.2x 35.7x
2 50 2500 emd2 (dense) 7.89 ms 1204.34 ms 615.41 ms 152.7x 78.0x
2 100 10000 emd2 (dense) 153.00 ms - - - -
3 5 125 emd2 (dense) 0.12 ms 1.46 ms 0.58 ms 12.0x 4.8x
3 8 512 emd2 (dense) 0.69 ms 38.65 ms 15.32 ms 56.1x 22.2x
3 10 1000 emd2 (dense) 2.02 ms 153.81 ms 62.38 ms 76.0x 30.8x
3 20 8000 emd2 (dense) 74.13 ms 22870.76 ms 10820.37 ms 308.5x 146.0x
3 30 27000 emd2 (dense) 902.89 ms - - - -
4 4 256 emd2 (dense) 0.32 ms 7.99 ms 3.05 ms 25.3x 9.6x
4 5 625 emd2 (dense) 1.27 ms 72.92 ms 29.14 ms 57.6x 23.0x
4 6 1296 emd2 (dense) 3.97 ms 351.34 ms 141.86 ms 88.4x 35.7x
4 10 10000 emd2 (dense) 129.96 ms - - - -

Notes:

  • - entries: dense/lazy skipped above n=2000 nodes to keep the benchmark's runtime bounded (full bipartite network simplex gets slow, and dense M becomes memory-prohibitive); emd_grid_l1 is still timed there to show scaling. The (3, 30) row (27,000 nodes) mirrors the 30×30×30 example from the discussion.
  • In 1D, the speedup over emd2_1d is modest (1.0x-3.5x) rather than the 1000x+ margin seen against the generic bipartite solvers — expected, since emd2_1d is already the right O(n log n) tool for a shared sorted support, and emd_grid_l1's own 1D plan path literally calls the same underlying routine. The gap that remains is emd_grid_l1's closed-form, network-simplex-free cost-only path.
  • For ndim >= 2, speedup over dense/lazy grows with n as expected, up to ~150x/300x at n in the low thousands.

Deliberately out of scope for this PR

This PR is scoped to L1 only, with no gradient support. If this direction is OK for integration, natural follow-ups would be:

Question: batched support?

Should emd_grid_l1 support a batch dimension (e.g. A, B of shape (B, n_1, ..., n_d))? The generic exact solvers in POT (ot.emd/ot.emd2/ot.emd2_lazy) don't batch — network simplex is a sequential pivoting algorithm per problem, so there's no way to vectorize it the way Sinkhorn/proximal iterations are batched in ot.batch. That's still true here: the Ling & Okada reduction is inherently per-pair and CPU-oriented. But since each pair in a batch is fully independent, one option specific to this solver would be to parallelize across the batch in C++ (e.g. an OpenMP parallel for over batch entries, each running its own NetworkSimplexSimple instance), rather than trying to vectorize within a single solve. Happy to prototype this as a follow-up if there's interest, but wanted to flag it now rather than bake in an API that doesn't anticipate it.

Test plan

  • pytest test/test_grid.py (32 tests: correctness vs. dense solver on 1D-4D grids, plan/coupling marginals, backend round-trips incl. GPU, dispatch mocking)
  • Doctest in ot/lp/_grid.py
  • Full existing test suite (pytest test/, excluding slow gromov/unbalanced/sliced/batch dirs): 1719 passed, no regressions
  • ruff check/ruff format clean
  • Manual cross-check against ot.emd2/ot.emd2_lazy/ot.emd2_1d on random grids up to 27,000 nodes (see benchmarks above)

Benchmarking script (not committed, for reference)

local_sandbox/bench_grid_l1.py
"""
Benchmark ot.lp.emd_grid_l1 against reference EMD solvers on random grid
histograms of increasing dimension and resolution.

For ndim >= 2, the references are ot.emd2_lazy and ot.emd2 (dense), the two
generic bipartite solvers emd_grid_l1 is meant to replace for this problem.
For ndim == 1, the fair reference is POT's own dedicated 1D solver,
ot.emd2_1d (an O(n log n) sort-based algorithm -- the closest existing POT
counterpart to emd_grid_l1's O(n) closed form in that case), so the generic
dense/lazy bipartite solvers are skipped there.

Not part of the test suite: a development artefact used to produce numbers
for the PR description. Run with:

    python local_sandbox/bench_grid_l1.py
"""

import time

import numpy as np

import ot
from ot.lp import emd_grid_l1

# (ndim, bins_per_dim) configs. Kept small for ndim >= 3 so the whole script
# runs in a couple of minutes; dense/lazy are skipped above SKIP_DENSE_ABOVE
# / SKIP_LAZY_ABOVE nodes to avoid blowing up the runtime, while emd_grid_l1
# is still timed there to show how far it scales.
CONFIGS = [
    (1, 10),
    (1, 100),
    (1, 1000),
    (1, 10000),
    (1, 100000),
    (2, 10),
    (2, 20),
    (2, 32),
    (2, 50),
    (2, 100),
    (3, 5),
    (3, 8),
    (3, 10),
    (3, 20),
    (3, 30),  # matches the 30x30x30 example in the GitHub discussion
    (4, 4),
    (4, 5),
    (4, 6),
    (4, 10),
]

SKIP_DENSE_ABOVE = 2000
SKIP_LAZY_ABOVE = 2000
REPEATS = 3

# Explicitly requested larger reference points (ndim >= 2): run dense/lazy
# here too even though they exceed the skip thresholds above. Single repeat
# only, since these are the expensive end of the range. The default
# numItermax=100000 is not enough for network simplex to converge on a full
# bipartite graph at this size, so it is raised here too.
FORCE_FULL = {(2, 50), (3, 20)}
FORCE_FULL_REPEATS = 1
FORCE_FULL_NUM_ITER_MAX = 2_000_000


def make_grid_coords(shape):
    axes = [np.arange(s) for s in shape]
    return (
        np.array(np.meshgrid(*axes, indexing="ij"))
        .reshape(len(shape), -1)
        .T.astype(np.float64)
    )


def random_histograms(n, rng):
    a = rng.rand(n)
    a /= a.sum()
    b = rng.rand(n)
    b /= b.sum()
    return a, b


def time_call(fn, repeats=REPEATS):
    times = []
    result = None
    for _ in range(repeats):
        t0 = time.perf_counter()
        result = fn()
        times.append(time.perf_counter() - t0)
    return result, min(times)


def bench_config(ndim, bins_per_dim, seed=0):
    shape = (bins_per_dim,) * ndim
    n = bins_per_dim**ndim
    rng = np.random.RandomState(seed)
    a, b = random_histograms(n, rng)
    A, B = a.reshape(shape), b.reshape(shape)
    forced = (ndim, bins_per_dim) in FORCE_FULL
    repeats = FORCE_FULL_REPEATS if forced else REPEATS

    row = {"ndim": ndim, "bins_per_dim": bins_per_dim, "n": n}

    numItermax = FORCE_FULL_NUM_ITER_MAX if forced else 100000
    row["numItermax"] = numItermax
    row["lazy_converged"] = None
    row["dense_converged"] = None

    row["grid_cost"], row["grid_time"] = time_call(lambda: emd_grid_l1(A, B))

    if ndim == 1:
        # For 1D, the fair "reference solver" is POT's own dedicated 1D
        # solver (also what emd_grid_l1's own 1D path calls under the hood
        # for the plan, via emd_1d_sorted) rather than the generic
        # dense/lazy bipartite network simplex. No lazy column here.
        row["dense_label"] = "emd2_1d"
        row["lazy_cost"], row["lazy_time"] = None, None
        x = np.arange(n, dtype=np.float64)
        t0 = time.perf_counter()
        row["dense_cost"], row["dense_time"] = time_call(
            lambda: ot.emd2_1d(x, x, a, b, metric="cityblock"), repeats=repeats
        )
        print(
            f"  [emd2_1d ndim=1 bins={bins_per_dim}] took "
            f"{time.perf_counter() - t0:.1f}s"
        )
        return row

    row["dense_label"] = "emd2 (dense)"

    if n <= SKIP_LAZY_ABOVE or forced:
        coords = make_grid_coords(shape)
        t0 = time.perf_counter()

        def _run_lazy():
            cost, lazy_log = ot.emd2_lazy(
                coords,
                coords,
                a,
                b,
                metric="cityblock",
                return_matrix=False,
                numItermax=numItermax,
                log=True,
            )
            row["lazy_converged"] = lazy_log["result_code"] == 1
            return cost

        row["lazy_cost"], row["lazy_time"] = time_call(_run_lazy, repeats=repeats)
        print(
            f"  [lazy  ndim={ndim} bins={bins_per_dim}] took "
            f"{time.perf_counter() - t0:.1f}s, converged={row['lazy_converged']}"
        )
    else:
        row["lazy_cost"], row["lazy_time"] = None, None

    if n <= SKIP_DENSE_ABOVE or forced:
        coords = make_grid_coords(shape)
        M = ot.dist(coords, coords, metric="cityblock")
        t0 = time.perf_counter()

        def _run_dense():
            cost, dense_log = ot.emd2(a, b, M, numItermax=numItermax, log=True)
            row["dense_converged"] = dense_log["result_code"] == 1
            return cost

        row["dense_cost"], row["dense_time"] = time_call(_run_dense, repeats=repeats)
        print(
            f"  [dense ndim={ndim} bins={bins_per_dim}] took "
            f"{time.perf_counter() - t0:.1f}s, converged={row['dense_converged']}"
        )
    else:
        row["dense_cost"], row["dense_time"] = None, None

    return row


def fmt_time(t):
    return "-" if t is None else f"{t * 1000:8.2f} ms"


def fmt_cost(c):
    return "-" if c is None else f"{c:10.6f}"


def main():
    rows = []
    t_start = time.perf_counter()
    header = (
        f"{'ndim':>4} {'bins':>5} {'n':>7} | "
        f"{'grid cost':>10} {'lazy cost':>10} {'dense cost':>10} | "
        f"{'grid time':>11} {'lazy time':>11} {'dense time':>11} | speedup(lazy/grid) speedup(dense/grid)"
    )
    print(header)
    print("-" * len(header))
    for ndim, bins in CONFIGS:
        row = bench_config(ndim, bins)
        rows.append(row)

        speedup_lazy = (
            "-"
            if row["lazy_time"] is None
            else f"{row['lazy_time'] / row['grid_time']:8.1f}x"
        )
        speedup_dense = (
            "-"
            if row["dense_time"] is None
            else f"{row['dense_time'] / row['grid_time']:8.1f}x"
        )

        print(
            f"{row['ndim']:>4} {row['bins_per_dim']:>5} {row['n']:>7} | "
            f"{fmt_cost(row['grid_cost'])} {fmt_cost(row['lazy_cost'])} {fmt_cost(row['dense_cost'])} | "
            f"{fmt_time(row['grid_time'])} {fmt_time(row['lazy_time'])} {fmt_time(row['dense_time'])} | "
            f"{speedup_lazy:>18} {speedup_dense:>19}  (reference: {row['dense_label']})"
        )

        # Sanity checks: costs must agree across solvers, but only when the
        # reference solver actually reports having converged -- an
        # unconverged network simplex returns a feasible but suboptimal
        # (too high) cost, which is expected, not a bug.
        if row["dense_cost"] is not None:
            if row["dense_converged"] is False:
                print(
                    f"  NOTE: dense did not converge (numItermax={row['numItermax']}); "
                    f"cost {row['dense_cost']:.6f} vs grid {row['grid_cost']:.6f} "
                    "(dense is expected to be >= grid's exact cost)"
                )
                assert row["dense_cost"] >= row["grid_cost"] - 1e-6
            else:
                np.testing.assert_allclose(
                    row["grid_cost"], row["dense_cost"], rtol=1e-5, atol=1e-7
                )
        if row["lazy_cost"] is not None:
            if row["lazy_converged"] is False:
                print(
                    f"  NOTE: lazy did not converge (numItermax={row['numItermax']}); "
                    f"cost {row['lazy_cost']:.6f} vs grid {row['grid_cost']:.6f} "
                    "(lazy is expected to be >= grid's exact cost)"
                )
                assert row["lazy_cost"] >= row["grid_cost"] - 1e-6
            else:
                np.testing.assert_allclose(
                    row["grid_cost"], row["lazy_cost"], rtol=1e-5, atol=1e-7
                )

    print(f"\nTotal wall time: {time.perf_counter() - t_start:.1f} s")

    print("\nMarkdown table:\n")
    print(
        "| ndim | bins/dim | n | reference solver | grid cost | lazy cost | "
        "reference cost | grid time | lazy time | reference time | lazy/grid | "
        "reference/grid |"
    )
    print("|---|---|---|---|---|---|---|---|---|---|---|---|")
    for row in rows:
        speedup_lazy = (
            "-"
            if row["lazy_time"] is None
            else f"{row['lazy_time'] / row['grid_time']:.1f}x"
        )
        speedup_dense = (
            "-"
            if row["dense_time"] is None
            else f"{row['dense_time'] / row['grid_time']:.1f}x"
        )
        lazy_mark = "&dagger;" if row["lazy_converged"] is False else ""
        dense_mark = "&dagger;" if row["dense_converged"] is False else ""
        print(
            f"| {row['ndim']} | {row['bins_per_dim']} | {row['n']} "
            f"| {row['dense_label']} "
            f"| {fmt_cost(row['grid_cost']).strip()} "
            f"| {fmt_cost(row['lazy_cost']).strip()}{lazy_mark} "
            f"| {fmt_cost(row['dense_cost']).strip()}{dense_mark} "
            f"| {fmt_time(row['grid_time']).strip()} "
            f"| {fmt_time(row['lazy_time']).strip()} "
            f"| {fmt_time(row['dense_time']).strip()} "
            f"| {speedup_lazy} | {speedup_dense} |"
        )
    print(
        "\n&dagger; did not fully converge within its numItermax "
        f"({FORCE_FULL_NUM_ITER_MAX}); cost shown is an upper bound on the "
        "true optimum (which emd_grid_l1's cost matches exactly)."
    )


if __name__ == "__main__":
    main()

🤖 Generated with Claude Code

Introduce ot.lp.emd_grid_l1(A, B), an exact solver for the Earth Mover's
Distance with the cityblock ground metric between histograms sharing a
d-dimensional Cartesian grid support. Rather than solving the min-cost
flow on the full bipartite graph (as ot.emd would), this reduces the
problem to a min-cost flow on the much sparser grid adjacency graph
following Ling & Okada (2007), then hands it to POT's existing
off-the-shelf network simplex LP solver instead of their bespoke
tree-based one.

- New ot/lp/sparse_digraph.h: a general (non-bipartite) sparse digraph,
  needed because every grid cell can carry both supply and demand,
  unlike the existing bipartite sparse_bipartitegraph.h.
- New EMD_wrap_grid_l1 in EMD_wrapper.cpp/EMD.h: builds the grid graph
  from a shape array, runs NetworkSimplexSimple, and decomposes the
  resulting flow into a transportation plan (including same-bin
  "self-mass" that already overlaps between A and B, which the flow
  decomposition alone would miss).
- New ot/lp/_grid.py: the Python-facing emd_grid_l1, with a dedicated
  1D fast path (POT's own emd_1d_sorted for the plan; a closed-form,
  fully backend-native O(n) reduction for the cost-only case, so a 1D
  GPU array never leaves the device). Backend-compatible throughout;
  the sparse plan is returned as G via the backend's coo_matrix, same
  convention as ot.emd2_lazy's return_matrix.
- test/test_grid.py: correctness against the dense solver, plan/coupling
  marginal checks, backend round-trips, and the 1D dispatch.

Deliberately scoped to L1 only, with no gradient support yet -- see the
PR description for benchmarks and open questions on L2 and batching.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.92%. Comparing base (98d09a1) to head (c35d037).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #863      +/-   ##
==========================================
+ Coverage   96.86%   96.92%   +0.05%     
==========================================
  Files         128      130       +2     
  Lines       26304    26755     +451     
==========================================
+ Hits        25480    25931     +451     
  Misses        824      824              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tvercaut and others added 4 commits September 18, 2026 14:46
test_emd_grid_l1_1d_direct_plan_helper_mass_mismatch exercises the
mass-mismatch early return in _emd_grid_l1_1d_plan, which codecov
flagged as uncovered.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ckends

Every other array-creation helper (zeros/ones/full) respects type_as for
both dtype and device, but arange() ignored it entirely for dtype (and
for device on Numpy/Jax/Tensorflow/Cupy). Fixes PythonOT#864.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Expose the dual potentials alpha/beta (the gradient of cost w.r.t. A, B)
in log, via LEMON's network-simplex node potentials for d >= 2 (a free
byproduct of the solve) and a closed form for 1D grids. Add a grad
argument ('envelope' by default, or None) to control whether the
non-free 1D gradient pass runs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tvercaut and others added 2 commits September 20, 2026 20:31
The fixed arange() (see the previous commit, PythonOT#864) now actually reads
type_as.dtype, which broke roll_cols' scalar-shifts doctest example: a
plain int has no dtype to propagate. Only pass shifts as type_as when
it is itself a backend array, as it always is at the two real call
sites in this module.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cost now carries the exact (envelope-theorem) gradient w.r.t. A/B, the
same way ot.emd2/ot.emd2_lazy do: detached from any graph A/B were part
of, then rewired explicitly via nx.set_gradients using the dual
potentials alpha/beta. For d >= 2 this is unconditional (the potentials
are a free byproduct of the network-simplex solve); for a 1D grid it
follows grad ('envelope' by default, or None to opt out), since there
computing alpha/beta is not free. Also reshape the multi-D alpha/beta
to A.shape/B.shape once, for both log and set_gradients, instead of
keeping them flat.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tvercaut added a commit to tvercaut/emdgrid that referenced this pull request Sep 22, 2026
decompose_flows

Fixes decompose_flows' mass-loss bug found while validating this switch:
its path-tracing walker compared residual supply/flow against exact zero,
which is safe for an exact/integer Value (LEMON, OR-Tools) but not for a
real-valued one, where a residual that should be exactly zero after many
prior subtractions can land a few ULPs to one side. An exact `> 0`/`<= 0`
comparison then stalls the walk on a pass-through node before it reaches
a genuine deficit, silently dropping that source's remaining surplus from
the plan instead of moving it -- confirmed as the cause of a ~4% mass
loss at 1000 nodes when Value=double (the previous commit's message).

Fixed by comparing against `eps = static_cast<Value>(1e-10)` instead of
zero throughout. For an exact/integer Value this truncates to exactly 0
(the original, exact behaviour, so no change for the currently-shipped
OR-Tools/LEMON solvers). For a real-valued Value it matches the exact
tolerance POT's own decompose_grid_flows (EMD_wrapper.cpp) uses for the
identical purpose, in the in-progress emd_grid_l1 solver
(PythonOT/POT#863) -- a grid-adjacency EMD-L1
solver built the same way as mcf_potlemon_l1, reusing POT's stock,
unmodified NetworkSimplexSimple.

With decompose_flows fixed, mcf_potlemon_l1 now runs on potlemon's
Value=double directly, matching POT's own instantiation, the same way
the previous commit did for emd_potlemon. Verified with a 10-seed stress
test on a 1000-node grid: cost agrees with mcf_lemon_l1 to ordinary
cross-algorithm precision (~3e-5 relative), and the extracted plan's row
and column margins now match the input histograms to machine precision
(~3e-15) instead of drifting by up to 4%.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tvercaut added a commit to tvercaut/emdgrid that referenced this pull request Sep 22, 2026
…real-valued supplies (#48)

* Fix potlemon's entering-arc pivot tolerance to use machine epsilon

The vendored network simplex used a single POTLEMON_EPSILON (1e-8) for
both the zero-supply feasibility check and the entering-arc pivot
decision. POT's actual upstream (and Bonneel's, from which POT is
itself forked) keep these distinct: a loose ~1e-8 for supply/flow
bookkeeping, and machine epsilon for the pivot decision. That split
was collapsed into one constant at this file's very first vendoring.

For small integer-cost problems this is normally harmless, since a
genuinely improving reduced cost is at least 1 and safely clears a
1e-8-scaled threshold. But when a node's dual potential still carries
residual influence from the initial big-artificial-cost basis,
epsilonBound() can be very large, and a 1e-8-scaled threshold stops
being negligible — verified experimentally (via an unlanded real-
valued-supply prototype for mcf_potlemon_l1/emd_potlemon) to let the
solver report OPTIMAL at a valid but non-optimal transport plan.

Splits the constant into POTLEMON_EPSILON (kept at 1e-8, for supply/
flow bookkeeping) and POTLEMON_PIVOT_EPSILON (machine epsilon, for the
pivot decision), matching POT's own network_simplex_simple.h. No
behavioural change for the currently-shipped integer-cost solvers, all
of which stay well clear of this threshold in practice — this closes a
latent correctness gap for a lurking failure mode the existing test
suite happens not to exercise.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Template the pivot epsilon on Cost instead of a hardcoded double

std::numeric_limits<Cost>::epsilon() is 0 for an exact/integer Cost by
the standard, so this collapses to a plain `< 0` pivot decision for
the currently-shipped int64_t solvers -- unmodified LEMON's exact
behaviour, with no fudge factor at all where none is needed. For a
floating Cost it keeps POT's own 10x-machine-epsilon safety margin.

This mirrors nbonneel's original (std::numeric_limits<Cost>::epsilon(),
POT's own upstream before it diverged to a fixed constant for all Cost
types) while keeping POT's empirically-tuned 10x headroom rather than
nbonneel's unscaled 1x, since we have no evidence 1x is enough and
POT's own usage suggests it isn't.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Drop the 10x pivot-epsilon multiplier; wrap POTLEMON_EPSILON too

Traced both constants back through nbonneel's and POT's history:

- The 10x factor (2.2204460492503131e-15) was never a deliberately
  validated choice for the pivot decision. nbonneel introduced it in
  his 2015 parallelization commit, then dropped it himself in the 2018
  "updated to a newer version of the algo by LEMON" rewrite -- the
  exact commit this file's provenance already cites as its lineage --
  replacing it with plain std::numeric_limits<Cost>::epsilon(). POT
  forked from the 2015-era code and never picked up that later
  cleanup, so POT's own network_simplex_simple.h still carries the old
  literal today. pivotEpsilon() now matches nbonneel's current (and
  more clearly load-bearing) form.

- POTLEMON_EPSILON (the separate, looser supply/flow tolerance) is
  real and needed, not legacy: POT introduced it to fix
  github.com/PythonOT/POT/issues/126, a user hitting a false
  INFEASIBLE on entirely valid torch.softmax-generated float64
  distributions, where a leftover artificial arc carried a
  rounding-noise residual that an exact `!= 0` check rejected. Wraps
  it in NetworkSimplexSimple::supplyEpsilon(), mirroring
  BlockSearchPivotRule::pivotEpsilon(), instead of using the bare
  constant directly at each call site.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Inline supplyEpsilon's value instead of a namespace constant

POTLEMON_EPSILON had exactly one remaining reader after the previous
commit wrapped it in supplyEpsilon(). Fold the value and its
explanatory comment directly into the function, matching how
pivotEpsilon() already carries its own value with no separate named
constant.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Switch emd_potlemon to real-valued supplies, matching POT's EMD_wrap_lazy

With the pivot-epsilon fix in place, emd_potlemon can now run potlemon's
NetworkSimplexSimple with Value=double directly, exactly like POT's own
EMD_wrap_lazy (EMD_wrapper.cpp): filter bins by strict positivity, hand
the solver the raw (not-quite-exactly-balanced) supply, and let its own
feasibility tolerance (supplyEpsilon) absorb the residual. This removes
the integer quantization and its `scale` parameter for this solver.

Root-caused via a literal side-by-side comparison against POT's and
Bonneel's actual algorithms: with the old, unfixed pivot epsilon, this
same change produced valid-but-non-optimal transport plans reported as
OPTIMAL (up to 30% too expensive) on several test cases. Reproducing
POT's exact approach (raw double Value, strict-positivity filtering,
no residual correction) against the *fixed* epsilon gives answers
matching emd_lemon to ordinary floating-point cross-algorithm
precision, confirming the earlier pivot-epsilon bug was the entire
cause, not anything in emd_potlemon's own supply handling. Verified
with a 20-seed stress test on a 512-node grid (both metrics, max
relative error ~6e-5) and a mass-conservation check on the extracted
plan (~1e-15).

mcf_potlemon_l1 is deliberately left on quantized int64_t supplies:
switching it the same way surfaces a second, independent bug -- its
shared decompose_flows plan-extraction loses ~4% of the mass at 1000
nodes (the returned cost is unaffected; only the post-hoc path-tracing
that turns arc flows into a transport plan is wrong). emd_potlemon
never calls decompose_flows (it extracts flows directly from the
solver's sparse flow map), so it is unaffected. Fixing decompose_flows
for a real-valued Value type needs its own dedicated work.

Loosens the emd_potlemon vs. emd_lemon test tolerances from 1e-9 to
1e-4: they are no longer solving the identical quantized LP, so exact
agreement is not the right bar, only ordinary cross-algorithm
precision.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Switch mcf_potlemon_l1 to real-valued supplies, with an eps-tolerant
decompose_flows

Fixes decompose_flows' mass-loss bug found while validating this switch:
its path-tracing walker compared residual supply/flow against exact zero,
which is safe for an exact/integer Value (LEMON, OR-Tools) but not for a
real-valued one, where a residual that should be exactly zero after many
prior subtractions can land a few ULPs to one side. An exact `> 0`/`<= 0`
comparison then stalls the walk on a pass-through node before it reaches
a genuine deficit, silently dropping that source's remaining surplus from
the plan instead of moving it -- confirmed as the cause of a ~4% mass
loss at 1000 nodes when Value=double (the previous commit's message).

Fixed by comparing against `eps = static_cast<Value>(1e-10)` instead of
zero throughout. For an exact/integer Value this truncates to exactly 0
(the original, exact behaviour, so no change for the currently-shipped
OR-Tools/LEMON solvers). For a real-valued Value it matches the exact
tolerance POT's own decompose_grid_flows (EMD_wrapper.cpp) uses for the
identical purpose, in the in-progress emd_grid_l1 solver
(PythonOT/POT#863) -- a grid-adjacency EMD-L1
solver built the same way as mcf_potlemon_l1, reusing POT's stock,
unmodified NetworkSimplexSimple.

With decompose_flows fixed, mcf_potlemon_l1 now runs on potlemon's
Value=double directly, matching POT's own instantiation, the same way
the previous commit did for emd_potlemon. Verified with a 10-seed stress
test on a 1000-node grid: cost agrees with mcf_lemon_l1 to ordinary
cross-algorithm precision (~3e-5 relative), and the extracted plan's row
and column margins now match the input histograms to machine precision
(~3e-15) instead of drifting by up to 4%.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Fix clang-tidy findings from the previous two commits

- readability-isolate-declaration: split combined vector declarations
  in the new stress tests into separate statements.
- readability-math-missing-parentheses: parenthesize seed*N+M in the
  stress tests' seed derivation.
- readability-redundant-typename: Simplex is no longer a dependent
  type in mcf_potlemon_l1/emd_potlemon now that Value is hardcoded to
  double rather than the enclosing function's CompScalar, so the
  typename disambiguator these needed while Value was still templated
  is now redundant.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants