Conversation
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 Report✅ All modified and coverable lines are covered by tests. 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_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>
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>
6 tasks
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 samed-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 (asot.emd/ot.emd2would), this reduces the problem to a min-cost flow on the much sparser grid adjacency graph (O(d*n)arcs instead ofO(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)NetworkSimplexSimpleLP 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 existingsparse_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_l1inEMD_wrapper.cpp/EMD.h: builds the grid adjacency graph directly from ashapearray, runsNetworkSimplexSimpleon it, and (only when requested) decomposes the resulting Beckmann-style arc flow into an explicit transportation plan — including the "self-mass"AandBalready 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,Bare passed as actuald-dimensional arrays (not flattened + a separate shape argument) so they naturally carry their own grid geometry.return_plan=Falseby 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.emd_1d_sorted, skipping the network-simplex setup entirely;nx.cumsum/nx.abs/nx.sum) and no CPU round-trip at all, verified end-to-end on an MPS GPU tensor.ot.backend). The general (ndim >= 2) path and the 1D-with-plan path do need a CPU round-trip for the compiled solver, same asot.emd/ot.emd2_lazy.log["G"], a sparse matrix built via the backend'scoo_matrix— the same mechanism and conventionot.emd2_lazy'sreturn_matrixalready 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 ofGmatchA/Bexactly, 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). Comparesemd_grid_l1againstot.emd2(dense) andot.emd2_lazyforndim2-4, and againstot.emd2_1d(POT's own dedicated 1D solver) forndim=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 needednumItermaxraised from the default 100000 to 2,000,000 to actually converge on the full bipartite graph at that size (noted below).Notes:
-entries: dense/lazy skipped aboven=2000nodes to keep the benchmark's runtime bounded (full bipartite network simplex gets slow, and denseMbecomes memory-prohibitive);emd_grid_l1is still timed there to show scaling. The(3, 30)row (27,000 nodes) mirrors the 30×30×30 example from the discussion.emd2_1dis modest (1.0x-3.5x) rather than the 1000x+ margin seen against the generic bipartite solvers — expected, sinceemd2_1dis already the rightO(n log n)tool for a shared sorted support, andemd_grid_l1's own 1D plan path literally calls the same underlying routine. The gap that remains isemd_grid_l1's closed-form, network-simplex-free cost-only path.ndim >= 2, speedup over dense/lazy grows withnas expected, up to ~150x/300x atnin the low thousands.Deliberately out of scope for this PR
This PR is scoped to L1 only, with
nogradient support. If this direction is OK for integration, natural follow-ups would be:(d+1)-partite graph construction of Auricchio et al. (also discussed in Faster exact EMD for distributions on shared multi-dimensional cartesian grids (with cityblock or sqeuclidean ground metric) #862), as a separate solver alongside this one.Gradient support, most likely via the envelope theorem using dual potentials ([EDIT: Gradient computation now supported]net.potential()isn't currently plumbed throughEMD_wrap_grid_l1), mirroring howot.emd/ot.emd2_lazydo it.Question: batched support?
Should
emd_grid_l1support a batch dimension (e.g.A,Bof 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 inot.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 OpenMPparallel forover batch entries, each running its ownNetworkSimplexSimpleinstance), 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)ot/lp/_grid.pypytest test/, excluding slow gromov/unbalanced/sliced/batch dirs): 1719 passed, no regressionsruff check/ruff formatcleanot.emd2/ot.emd2_lazy/ot.emd2_1don random grids up to 27,000 nodes (see benchmarks above)Benchmarking script (not committed, for reference)
local_sandbox/bench_grid_l1.py🤖 Generated with Claude Code