Repository navigation
[MRG] Fix sparse simplex projection with oversized budgets - #876
Open
AHMETHAKANBEZIR1 wants to merge 4 commits into
Open
AHMETHAKANBEZIR1 wants to merge 4 commits into
AHMETHAKANBEZIR1 wants to merge 4 commits into
Conversation
Cap the effective row budget at its width, preserving the documented equivalence to the unconstrained projection. Add an analytic regression for exact and oversized budgets across all supported axis modes. Co-authored-by: Codex <noreply@openai.com>
…oversized-budget # Conflicts: # RELEASES.md
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #876 +/- ##
==========================================
- Coverage 97.00% 95.54% -1.46%
==========================================
Files 128 128
Lines 26349 26365 +16
==========================================
- Hits 25559 25191 -368
- Misses 790 1174 +384 🚀 New features to boost your workflow:
|
Fix TensorFlow CI failures without indexed tensor mutation and cover vector masses.
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.
Types of changes
Bug fix: use ordinary simplex projection for an unconstrained sparsity budget, focused regressions, and a release note.
Motivation and context / Related issue
Fixes #875. The documented behavior for max_nz greater than the projected dimension is ordinary simplex projection. Instead, the row slice only contains the actual width while the threshold index array has length max_nz, producing a broadcasting error. When the budget is at least the row width, use the existing ordinary simplex projection directly; axis=0 and axis=None inherit this through their existing recursion. This avoids indexed mutation on TensorFlow tensors. Smaller budgets retain the sparse projection path.
How has this been tested
Built POT's native extensions from this source checkout using MSVC 2022, OpenMP, Cython and the repository's Eigen submodule on Windows/Python 3.12.
On unmodified master 98d09a1, the added regression yields 3 failures/3 controls with NumPy. Restoring that exact baseline function in memory in a second environment yields 6 failures/6 controls across NumPy and PyTorch CPU. No source file is altered by that baseline harness.
After the fix, the complete test/test_utils.py module passes: 120 tests in the NumPy environment; 182 tests (2 warnings) in the NumPy/PyTorch 2.10 CPU environment. Expected values are analytic simplex solutions, covering all three axis modes with budgets equal to or larger than the projected dimension.
Configured pre-commit hooks applicable to all three changed files pass; git diff --check passes. Hooks reporting no matching files are not counted as executed checks.
Focused Sphinx autodoc/Napoleon build of the changed public API passes with warnings treated as errors. The harness references the existing algorithm footnote; its first isolated build warned that the existing footnote was unreferenced. The repository docstring is unchanged.
Not run: full package suite, full project documentation/gallery, GPU, or the TensorFlow/JAX/CuPy backends. Zero/negative sparsity-budget validation is outside this change.
PR checklist
I have read the CONTRIBUTING document.
The documentation is up-to-date with the changes I made (check build artifacts). The documented contract already states this behavior; only the focused API build was run locally.
All tests passed, and additional code has been covered with new tests. The complete utility module and regression passed; the full package suite was not run.
I have added the Issue fix to RELEASES.md.
AI assistance disclosure
This contribution was investigated, implemented and validated autonomously with Codex assistance on behalf of AHMETHAKANBEZIR1. It has not received independent human code review. The commit records Codex as a co-author. Maintainer review is requested; local test evidence is not a claim of upstream CI success.
Current base validation
Updated against current master (61e13ba), preserving both release-note entries. test/test_utils.py: 120 passed.
TensorFlow CI follow-up
The initial budget cap exposed the existing indexed-assignment path on TensorFlow: the six new scalar-budget cases failed with EagerTensor item assignment errors. The unconstrained-budget path now delegates to
proj_simplex, as the documented contract permits. Added vector-mass tests for both projection axes.