Skip to content

[MRG] Preserve weighted low-rank plan shapes under integer indexing - #893

Open
ktz03 wants to merge 2 commits into
PythonOT:masterfrom
ktz03:fix/weighted-lowrank-integer-indexing
Open

ktz03 wants to merge 2 commits into
PythonOT:masterfrom
ktz03:fix/weighted-lowrank-integer-indexing

Conversation

@ktz03

@ktz03 ktz03 commented Oct 19, 2026

Copy link
Copy Markdown

Types of changes

Bug fix.

Motivation and context / Related issue

Fixes #892.

For get_lowrank_lazytensor(Q, R, d), integer row indexing currently returns (1, m) instead of (m,), and indexing a single entry returns (1,) instead of (). This also affects the lazy plans returned by lowrank_sinkhorn.

Broadcast the diagonal weights directly against the indexed factor, preserving its rank. Parameterized regressions compare shapes and values against the dense product for weighted/unweighted tensors, positive/negative integer indices, mixed indices, columns, blocks, and full matrices. Existing full-plan computations keep the same values.

How has this been tested (if it applies)

  • python -m pytest test/test_utils.py test/test_lowrank.py test/test_solvers.py -q: 890 passed, 4 skipped, with NumPy and PyTorch CPU.
  • The focused 34-case subset fails 10 shape checks on unchanged production source and passes after the fix; it is included in the 890 tests above.
  • Independently checked additional public indexing cases and PyTorch gradients against dense products. Normal indexing controls retain their values and shapes.
  • Official pre-commit hooks pass for the changed files.

Windows 11, Python 3.12.14, NumPy 2.5.4, SciPy 1.18.1, PyTorch 2.14.1. Existing transpose/convergence warnings remain. The full repository suite, CUDA, JAX, TensorFlow, documentation build, and performance benchmarks were not run.

PR checklist

  • I have read the CONTRIBUTING document.
  • The documentation is up-to-date with the changes I made (check build artifacts).
  • All tests passed, and additional code has been covered with new tests.
  • I have added the PR and Issue fix to the RELEASES.md file.

@ktz03
ktz03 marked this pull request as ready for review October 19, 2026 03:50

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.

Weighted low-rank LazyTensor integer indexing keeps an extra dimension

1 participant