feat: unlabeled-operand pairing by size (#736)#747
Merged
Conversation
) Implements the coordinate-alignment intro of the v1 convention: numpy / list / polars operands carry no labels, so their axes pair with the linopy operand's dimensions by size. Ambiguity (two dims share a size, or a square array) or no size match raises under v1, pointing to wrap-in-a-DataArray; legacy keeps the positional pairing and warns when the v1 result would differ or reject. Core (linopy/alignment.py): - _pair_axes_by_size + _dims_for_unlabeled_operand: the size-pairing, with the legacy/v1 fork. - as_constant: normalize degenerate operands on entry — a Python list becomes a numpy array (lists have no numeric operators), a 0-d array unwraps to a scalar (so it takes the scalar fast-path, not pairing). - _broadcast_to_coords gains unlabeled_pairing="semantic" for the arithmetic (strict=False) path; explicit-coords callers stay positional. - Two conversion fixes the seam exposed: as_dataarray's scalar branch and the positional fallback now exclude HELPER_DIMS, so a scalar never broadcasts over _term/_factor. Operators (linopy/expressions.py): the binary dunders call as_constant on entry and drop the dims=coord_dims hint so unlabeled operands reach the size-pairing; matmul pairs the contracted dim by size too. Variable operators inherit this via delegation. UNLABELED_TYPES is the single source of truth for the dispatch. Tests: TestUnlabeledPairing (test_legacy_violations) — parametrized over numpy/list/polars with legacy/v1 markers; the two pre-existing unlabeled-rhs constraint tests forked legacy/v1. benchmark_model example names its rhs axis (it used an ambiguous square-dim rhs). Docs: convention.md §736 TODO resolved; legacy-removal.md lists the positional-pairing fallback. Full suite under both semantics: 6456 passed, 546 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…thmetic Per review: there's no principled reason add_variables / add_constraints bounds should pair an unlabeled array positionally while arithmetic operands pair by size. Positional was a coords-as-truth carryover from #732, and it's strictly worse — it errors on cases size-pairing resolves (`lower=np.arange(5)` against dims (a:4, time:5) now picks `time` instead of conflicting on `a`) and silently guesses where size-pairing safely raises. So pairing is now by size *everywhere*, unconditionally: - `_broadcast_to_coords` drops the strict/positional gate; the `strict` parameter goes back to meaning only "raise vs pass-through on mismatch". - 0-d arrays skip pairing (no axes); they broadcast as scalars. - The conversion is handed the normalized `expected` dict rather than the raw sequence-form `coords`, so coords filter by name — a sequence-form `coords` would otherwise zip dims to coords positionally and mis-assign once pairing chose a non-leading dim. Tests: TestUnlabeledPairing gains add_variables bound cases (size-pair + ambiguity raise). Full suite under both semantics: 6458 passed. The convention intro already states the by-size rule without an add_variables carve-out; _dims_for_unlabeled_operand's docstring updated to say "input" (bounds/masks/operands), not "arithmetic operand". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…iew fixes Address review of #747: - Bug: to_constraint lost its docstring — `rhs = as_constant(rhs)` was inserted before the docstring, demoting it to a dead expression (to_constraint.__doc__ was None). Moved the call after the docstring. - _broadcast_to_coords is now a 6-step pipeline; each phase is a named helper with a focused docstring (_label_input, _reindex_reordered_dims, _expand_missing_dims, _order_like_coords, _dims_for_positional_input), replacing the inline comment paragraphs. - Deduplicate the matmul operand conversion: both LinearExpression and QuadraticExpression __matmul__ call the new alignment.matmul_operand_to_dataarray (one home for the contraction pairing). - _dims_for_unlabeled_operand's legacy branch returns `positional` (len(shape) names), symmetric with the v1 `paired` return, instead of the full candidate list trimmed implicitly downstream. - test: TestUnlabeledPairing.test_v1_pairs_by_size uses a stacked @pytest.mark.v1 decorator instead of post-hoc mark reassignment. Full suite under both semantics: 6458 passed, 548 skipped. mypy + pre-commit clean; restored docstring passes --doctest-modules. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The xr.DataArray(naxis, dims=["dim_0"]) wrapping is self-explanatory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…_positional_input Review follow-ups: - matmul_operand_to_dataarray -> _matmul_operand_to_dataarray (internal helper, consistent with _dims_for_unlabeled_operand's underscore prefix). - It no longer re-implements the "is it unlabeled? -> pair by size" decision; it calls _dims_for_positional_input, the single owner of "which dims an unlabeled operand's axes adopt". Since its expected dict is built from coord_dims (no helper dims), the fallback is exactly list(coord_dims), so behavior is unchanged. The two call paths now differ only in what they should: matmul converts raw, the broadcast pipeline runs reindex/expand/transpose. Full suite under both semantics: 6458 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both branches of "infer dim order only when the user didn't name it" are now covered: - no dims + unlabeled -> size-pair (existing TestUnlabeledPairing tests); - explicit dims -> honored positionally, like xarray, even when size-pairing would be ambiguous (new test_explicit_dims_bypass_size_pairing). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ards) The size-pairing unification also runs under legacy (positional + a deprecation warning), so legacy needs explicit coverage that results are unchanged and the warnings fire correctly: - Rename test_legacy_positional_with_warning -> test_legacy_no_divergence_ is_silent: it never asserted a warning — it's the no-divergence case, now pinned as silent (LinopySemanticsWarning escalated to error). - test_legacy_warns_when_v1_would_differ: tighten the match to "pairs by size instead" (the divergence message, not the ambiguity one). - NEW test_legacy_ambiguous_pairs_positionally_with_warning: the square (p:4, q:4) case where v1 raises — legacy must pair positionally with the leading dim and warn, never raise. The strongest legacy/v1 divergence guard. - NEW test_legacy_add_variables_bound_positional: the unification touched add_variables; pins that legacy still assigns an unlabeled bound positionally, producing the pre-#736 result. Full suite under both semantics: 6461 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review + follow-up questions:
- Cross-type uniformity is now proven, not just asserted: a shared
UNLABELED_1D parametrization (numpy / list / polars) covers the raise
and matmul paths too, not only the happy-path pairing test. Removes the
inline lambda duplication.
- Multi-dim numpy coverage added:
- 2-d square (4, 4) vs (p:4, q:4) — the (a,b)-vs-(b,a) ambiguity the
convention calls out; exercises the multi-axis-same-size branch of
_pair_axes_by_size that 1-d cases never reach.
- 2-d (5, 3) no-size-match raises.
- lower-rank operand against a 4-dim variable: a 2-d (4, 5) pairs its
axes with the (b, c) subset by size and broadcasts over (a, d).
- an ambiguous axis within a higher-rank operand still raises.
- Tighten the legacy divergence test's suppress(Exception) ->
suppress(ValueError) so it pins the expected shape-conflict.
Full suite under both semantics: 6471 passed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Cosmetic reorg (no behavioral change, same 24 tests): - Move the `wide` fixture up beside `xy` / `square` so all three fixtures sit together instead of stranded mid-class. - Order the v1 tests by surface with section comments: 1-d operands → multi-dim → matmul → construction. The 1-d ambiguity/no-match tests now sit next to their multi-dim counterparts rather than interleaved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
convention.md specifies the v1 convention; that legacy keeps the old behavior and warns on divergence is the universal legacy-bridge pattern, not specific to unlabeled pairing — no need to restate it here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
It is a runtime isinstance tuple, the same category as CONSTANT_TYPES, so it belongs with the other type definitions, not as a module-local global in alignment.py. Derive it from an UnlabeledLike alias via get_args, mirroring ConstantLike/CONSTANT_TYPES. Left unannotated so isinstance narrowing is preserved at alignment.py's np.shape/np.ndim call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Closes #736. Stacked on #717 (
feat/arithmetic-convention).Implements the last unimplemented rule of the v1 convention's coordinate-alignment intro: unlabeled operands pair with the linopy operand's dimensions by size.
The rule
numpy arrays, lists, and polars Series carry no dimension labels, so their axes adopt the operand's dims by size — the same rule for arithmetic operands and for
add_variables/add_constraintsbounds and masks (no positional carve-out for construction):Ambiguity raises (v1), pointing at the explicit fix:
The rule is uniform:
add_variables(coords=[a:4, time:5], lower=np.arange(5))now resolveslowertotime(positional pairing errored ona); a square/ambiguous bare-numpy bound raises asking for aDataArraywrap, instead of silently guessing.Implementation (
linopy/alignment.py)_pair_axes_by_size+_dims_for_unlabeled_operand— the size-pairing with the legacy/v1 fork.as_constant— normalizes degenerate operands on entry: a Pythonlist→ numpy array (lists have no numeric operators), a 0-d array → Python scalar (takes the scalar fast-path, never pairs).ConstantLikestays numeric-only._broadcast_to_coordsgainsunlabeled_pairing="semantic"for the arithmetic path; explicit-coords callers (add_variables) stay positional.as_dataarray's scalar branch and the positional fallback now excludeHELPER_DIMS, so a scalar never broadcasts over_term/_factor. (HELPER_DIMSwas already the global registry;_groupis transient and correctly excluded.)Why
dims=self.coord_dimswas dropped from the operator callsThe arithmetic operators (
expressions.py_add_constant,_apply_constant_op,to_constraint;variables.pyto_linexpr; both__matmul__) previously passeddims=self.coord_dimsintobroadcast_to_coords. That argument pinned an unlabeled array's axes to those dims by position — which is exactly the behavior #736 replaces. While it's passed, size-pairing can never run (an explicitdimsmeans "the caller named the axes").So every arithmetic call site drops it. This is safe because
coords=self.coordsalready carries the same dim information, anddimsonly ever affected unlabeled inputs:dimsentirely (they carry their own labels) — unchanged._dims_for_unlabeled_operandand pair by size — the point of the PR.dims=self.coord_dimsdid do one extra thing worth calling out: it excluded helper dims (_term,_factor) from the positional labeling. Dropping it surfaced three latentHELPER_DIMSleaks (a scalar gaining_term, etc.), now fixed in the conversion layer (as_dataarray's scalar branch and the positional fallback both excludeHELPER_DIMS).as_constanton entry andUNLABELED_TYPESas the single dispatch source complete the picture; Variable operators inherit it all via delegation.Tests
TestUnlabeledPairing(test_legacy_violations) — parametrized over numpy / list / polars, with@pytest.mark.legacy/@pytest.mark.v1pairs covering pair-by-size, order-independence, ambiguity, no-match, the DataArray escape hatch, and matmul.benchmark_modelexample named its rhs axis (it used an ambiguous square-dim rhs — now demonstrates the convention).Verification
Full suite under both semantics: 6458 passed, 548 skipped. mypy + pre-commit clean. convention.md §736 TODO resolved; legacy-removal.md updated.
🤖 Generated with Claude Code