feat: v1 semantic convention#717
Draft
FBumann wants to merge 65 commits into
Draft
Conversation
The design goals and transitioning goals for linopy's v1 arithmetic convention, under arithmetics-design/goals.md. The convention itself and the bug catalogue (meta issue #714) follow separately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Placeholder for the v1 convention document, to be written. Goals are in arithmetics-design/goals.md; the bug catalogue is the meta issue #714. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
FBumann
force-pushed
the
feat/arithmetic-convention
branch
from
40af9d6 to
1e336a4
Compare
Flesh out convention.md from the placeholder into the full spec — thirteen numbered sections in three groups: absence (§1–§7), coordinate alignment (§8–§11), and constraints and reductions (§12–§13). Covers the strict exact-match alignment model and the propagate-don't-fill NaN/absence convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The convention governs coordinate alignment, absence/NaN handling, constraints, and reductions — not just arithmetic operators — so retitle convention.md and goals.md to "The v1 convention". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduce linopy.options["semantics"] — legacy (default) or v1 — with LinopySemanticsWarning, a FutureWarning shown to users by default and exported at top level. Add the autouse `semantics` conftest fixture that runs every test under both conventions, plus legacy/v1 markers to pin a test to one. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 22, 2026
Merged
`_align_constant` branches on `options["semantics"]`: v1 uses exact alignment via `xr.align(join="exact")`; legacy keeps the size-aware positional/left-join behaviour and emits `LinopySemanticsWarning` when v1 would diverge. `_add_constant`/`_apply_constant_op` raise on a NaN in a user-supplied constant under v1, warn under legacy. `Variable.__mul__(DataArray)` now routes through `to_linexpr() * other` so the LinearExpression checks fire; the scalar fast-path is preserved (a NaN scalar diverts to the expression path so v1 raises). Marks the bug-class test groups `TestCoordinateAlignment` (#708/#586/ #550), `TestConstraintCoordinateAlignment`, `TestNaNMasking`, `test_auto_mask_constraint_model`, and four piecewise NaN-padding tests as `@pytest.mark.legacy` — they assert the very behaviour v1 forbids. v1 coverage of those bug classes accretes via later slices. `test/test_legacy_violations.py` (new) adds 22 paired tests covering §5/§8/§9 plus the PyPSA #1683 `0*inf=NaN` case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`merge` now pre-validates that all operands agree on the labels of every shared *user* dimension before concatenating. Helper dims (`_term`, `_factor`) and the concat dim itself are excluded — those legitimately vary between operands. v1 raises on mismatch; legacy keeps current size-based override/outer behaviour and emits `LinopySemanticsWarning` when v1 would diverge. The check uses a new `_merge_shared_user_coords_differ` helper. The existing override/outer decision is unchanged for the actual `xr.concat` call — the new check only gates whether legacy/v1 accept the merge, never how the concat itself runs. Adds 8 paired tests for var+var, var-var, expr+expr, broadcast guard, and warning emission on the merge path. Reclassifies as `@pytest.mark.legacy`: `test_non_aligned_variables` (deliberately disjoint coords), `test_linear_expression_sum` / `test_linear_expression_sum_with_const` (assert `v.loc[:9]+v.loc[10:]` merges), `TestJoinParameter` cases that build `a*b` from mismatched- coord vars, and two SOS2 reformulation tests. File-level legacy mark on `test_piecewise_constraints.py` + `test_piecewise_feasibility.py` until `linopy/piecewise.py` itself is made v1-aware (tracked as Slice P). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Variable.to_linexpr() now produces a LinearExpression whose absent slots (labels == -1) carry NaN coeffs and NaN const under v1, so downstream arithmetic has something to propagate. The expression constant operators (_add_constant, _apply_constant_op) no longer fillna(0) self.const / self.coeffs under v1 — NaN flows through. `merge` sums const along _term with skipna=False under v1, so a slot that's absent in any operand stays absent in the result. Legacy paths keep the silent-fill behaviour verbatim. LinearExpression.isnull() now returns `const.isnull()` under v1: a slot is absent iff its const is NaN. ``vars == -1`` is a dead-term signal (the slot can still be a present constant after fillna), not a slot-level absence marker. Legacy keeps the historical ``(vars == -1).all() & const.isnull()`` formula for byte-for-byte compatibility. Variable.fillna(numeric) now returns a LinearExpression (a constant isn't a variable). Variable.fillna(Variable) stays Variable, as before. Adds 11 tests for §6 propagation (mul/add/sub/div preserve absence, absent-vs-zero distinguishable, present + absent propagates) and §7 resolution (fillna numeric on expr / Variable, present-zero revival). Reclassifies test_masked_variable_model as @pytest.mark.legacy — its assertion "x bound to 10 at masked-y slots" only holds because legacy collapses absent y to 0. The v1 way is x + y.fillna(0) >= 10; a counterpart test in test_legacy_violations.py pins this. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The convention spec names ``reindex`` and ``reindex_like`` among the absence-creating mechanisms (alongside ``mask=``, ``.where()``, ``.shift()``, and ``.unstack()``), but master only had them on ``LinearExpression``. Add them on ``Variable``, with the sentinel fill values (``labels=-1``, ``lower=upper=NaN``) so new positions slot cleanly into §6 propagation. The methods work the same way under both semantics — under legacy the sentinels exist but downstream arithmetic still collapses them back to 0 (the #712 bug), so the user-visible effect of reindex-as- absence only really lands under v1. Adds 5 tests: extend with absent, subset drops, reindex_like with another Variable, and the §4 + §6 hand-off (a reindex-introduced absent flows through ``* 3`` and is visible via ``isnull()``). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Slice C propagated NaN const cleanly but left the storage half-absent after a merge: `(1*x) + xs` at the absent slot kept the `1*x` term's valid coefficient and label even though `const` was NaN there. The §1/§2 promise "absence is one concept, whatever the dtype" only holds if `const.isnull()` at a slot ⇒ every term at that slot has `coeffs = NaN`, `vars = -1`. Add `_absorb_absence(ds)` and call it at the end of `merge` under v1. The constant-operand paths (`_add_constant`, `_apply_constant_op`) don't need explicit absorption — their NaN-propagation naturally preserves the invariant when the input is already v1-compliant (NaN * anything = NaN; dead terms stay dead). Only `merge` opens the gap by concatenating one operand's live term with another operand's absent slot along `_term`. `convention.md` §2 now states the invariant explicitly and introduces the *dead term* terminology, so `fillna(value)` reviving a slot while leaving the sentinel term in place reads as a feature, not a glitch. Adds `test_outer_fillna_then_add_collapses_to_just_added` pinning `(x + y.shift()).fillna(0) + x` — at the previously-absent slot the result has exactly one live term (`1·x[0]`) with `const = 0`, algebraically equal to `x[0]`. At present slots all three terms stay live (`2·x[i] + y[i-1]`), so fillna placement is load-bearing — moving it inside (`x + y.shift().fillna(0) + x`) would double-count `x` at the absent slot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`.add/.sub/.mul/.div/.le/.ge/.eq` already accepted a `join=` argument; this slice's job is just §12's RHS handling under v1. `to_constraint` branches on `options["semantics"]`. Under v1 it skips the legacy `reindex_like(self.const, fill_value=NaN)` step that silently padded a subset RHS, so a coord mismatch with the LHS now flows through `self.sub(rhs)` and gets caught by §8's exact alignment. A NaN in a user-supplied constant RHS raises at construction (§5) — including the PyPSA #1683 case of `min_pu * nominal_fix` with `p_nom=inf` and `p_min_pu=0`. An absent slot in the LHS (propagated from §6) still produces a NaN RHS at that row; downstream auto-mask drops the constraint there, which is exactly §12's "absent slot yields no row." Legacy keeps the old auto-mask path verbatim and adds a `LinopySemanticsWarning` whenever a NaN RHS is observed, so users get the rollout signal without behaviour change. Adds 11 paired tests: TestNamedMethodJoin (inner/outer/left across .add/.mul/.le, plus a "bare op still raises" guard) and TestConstraintRHS (subset RHS raises, NaN RHS raises, PyPSA #1683 on the constraint side, §6→§12 hand-off where the absent LHS slot yields NaN RHS, plus the paired legacy auto-mask documentation and warning-emission tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Coming next, in order:
Then a final pass on docs (user-facing migration / rollout — deferred so far). |
Three internal patterns were violating §8 / §11:
1. ``_add_incremental`` in ``linopy/piecewise.py`` builds
``delta_hi <= delta_lo`` from two ``.isel(piece_dim=slice)`` slices
of the same variable. ``drop=True`` is a no-op for slice indexers
so ``piece_dim`` stays on both with *different* labels (first n-1
vs last n-1 of piece_index) — v1 §8 rejects. Relabel the high
slice onto the low slice's labels so the comparison aligns by
label (the explicit-positional path of §10). Same fix for
``binary_hi <= delta_lo``.
2. ``_incremental_weighted`` computes ``bp0 = bp.isel({dim: 0})``
without ``drop=True``, leaving the breakpoint dim as a scalar
coord on the resulting expression. When that expression appears
as the RHS of ``links.eq_expr == ...`` it conflicts with the LHS,
which has no such coord — §11 aux-coord conflict. Add ``drop=True``.
3. ``reformulate_sos2`` builds its first/last constraints from
scalar isels at different positions on ``sos_dim`` (``x``/``M`` at
``n-1`` paired with ``z`` at ``n-2``, etc.). All without
``drop=True``, so the scalar ``sos_dim`` coord differs across
operands — §11 aux-coord conflict. Add ``drop=True`` to all three
sites.
Removes the module-level ``pytestmark = pytest.mark.legacy`` from
``test_piecewise_constraints.py`` and ``test_piecewise_feasibility.py``
and the method-level marks from the two SOS2 multidim tests. Suite is
+598 tests under v1 vs Slice E (legacy → v1 broadened coverage),
0 failures under either semantics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
§13 falls out of xarray's ``skipna=True`` default; no code changes needed. Adds 4 tests so future drift is caught: sum over a dim, sum without a dim, sum of all-absent (the zero expression), and groupby.sum across heterogeneously-present groups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `_conflicting_aux_coord(datasets)` and wires it into both `merge` and `_align_constant`. When two operands carry an aux coord of the same name with disagreeing values, v1 raises with a pointer to the explicit resolutions (``.drop_vars(...)`` or ``.assign_coords(...)``). xarray silently drops the conflict — the #295 bug — and legacy keeps that behaviour but now emits a `LinopySemanticsWarning`. The helper guards against string-dtype coord values (no `equal_nan=True` there) so the multiindex case keeps working. `_merge_shared_user_coords_differ` refactored to compare bare ``d.indexes[k]`` instead of ``d.coords[k]``: aux coords no longer leak into the §8 check, so §11 owns aux-coord conflicts cleanly and §8 owns dim-coord mismatches with a separate message. Convention §11 expanded from one paragraph: aux coords are validated and propagated but never computed with — they describe the data, they don't enter the math. Goal #4 in `goals.md` picks this up: user-attached auxiliary coordinates are the user's, linopy never silently rewrites them. `test_linear_expression.py::test_merge` adds ``drop=True`` to its ``.sel`` setup — the test was leaving a leftover scalar coord that v1 now correctly catches as a §11 conflict; the fix preserves the test's intent of exercising merge with differing term counts. Conflict-raising tests (TestAuxCoordConflict) cover expr+const, var+var, scalar-isel-without-drop, the ``drop=True`` escape hatch, plus the paired legacy left-wins documentation and warning-emission tests. Propagation guarantees land in a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Regression coverage on the half of §11 that wasn't tested before: non-conflicting aux coords carry through every binary operator and into constraints. xarray already preserves them; the tests guard against future drift (e.g. a reduction or helper accidentally dropping a non-dim coord). TestAuxCoordPropagation covers ``3*v``, ``v+5`` (single-operand, fast paths), ``v+v`` with matching aux (the merge path), ``v<=10`` (the constraint path), ``x*a`` / ``x+a`` / ``x/a`` / ``x<=a`` where only the constant DataArray carries the coord (the ``_align_constant`` path), and the var+var case where only one side has the coord. Together: every operator times every "one side / both sides" arrangement, since only conflicts on both sides raise. Runs under both semantics — the legacy behaviour matches the v1 behaviour for the non-conflict cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… solve Fills the convention-coverage gaps surfaced by review of the branch: - §1/§2 dead-term storage invariant: pin that after a merge with an absent slot, coeffs=NaN AND vars=-1, not just const=NaN. The existing propagation tests read through isnull() which only checks const, so a regression in _absorb_absence would have passed them. Multi-operand variant catches binary-only-absorption regressions. - §12 equality: mirror the existing <=/>= TestConstraintRHS coverage for ==. Subset RHS raises, NaN RHS raises, absence in LHS drops the row. - §11 extra operators: add mul-constant and == constraint cases to the existing TestAuxCoordConflict. The class already covered +-constant and var+var; these extend coverage to the other call-site shapes. - §13 scope note: mean/resample/coarsen aren't yet on LinearExpression (tracked in #703); the spec text is the rule those will follow when implemented. Docstring note in TestReductionsSkipAbsent makes this explicit so the gap doesn't read as missing coverage. - End-to-end v1 solve: test_masked_variable_model_v1_drops_constraint pins the v1 outcome at the solver layer — con0 masked at absent slots (solver-independent) and x bound to 0 where the constraint still binds. _v1_fillna_binds confirms the §7 escape hatch recovers the legacy outcome. Catches the regression where v1 silently produces wrong solutions instead of raising. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulls the seven v1-specific helpers and the user-NaN message out of ``expressions.py`` and into a dedicated ``linopy/semantics.py`` module — a single home for "what v1 means" that imports cleanly from ``config`` and ``constants`` only. Adds a tiny ``is_v1()`` predicate so the 16 scattered ``options["semantics"] == V1_SEMANTICS`` checks collapse to a one-line call. Helpers (renamed to drop the leading underscore now that they're a real module API): ``check_user_nan_scalar``, ``check_user_nan_array``, ``dim_coords_differ`` (was ``_shared_coords_differ`` — clearer name, matches ``merge_shared_user_coords_differ``), ``merge_shared_user_coords_differ``, ``conflicting_aux_coord``, ``absorb_absence``, plus ``is_v1``. No behaviour change — same checks, same warnings, same raises. The diff is mechanical: imports flipped, two local ``is_v1 = options[...]`` bindings replaced by the imported predicate, one missed ``_USER_NAN_MESSAGE`` reference in ``to_constraint`` routed through ``check_user_nan_array`` for consistency. ``expressions.py`` shrinks by ~105 lines. Future v1-only API surface (e.g. exposing ``is_v1()`` as ``linopy.is_v1()`` for downstream code) and the eventual legacy removal at 1.0 both reduce to deletions of ``semantics.py`` and its import sites. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three test clusters in ``test_legacy_violations.py`` had near-identical
``test_add_X``, ``test_mul_X``, ``test_div_X`` triples that varied only
by which binary operator they exercised. Collapse each into a single
``@pytest.mark.parametrize("op", ...)`` test:
- TestExactAlignmentConstant: same-size-different-labels and
subset-constant raises, parameterized over add/sub/mul/div.
- TestUserNaNRaises: NaN-DataArray raises over add/sub/mul/div, NaN
scalar over add/sub/mul (div scalar shares the same ``_apply_constant_op``
code path as mul, but ``x / nan`` trips ``__div__``'s unary-negate
TypeError before our check fires; the dispatch needs a separate
fix that's not worth pulling into this refactor).
- TestAbsencePropagation: ``shifted OP scalar`` preserves absence,
parameterized over add/sub/mul/div. Adds a per-op present-slot
value check so the parameterization broadens rather than narrows
the assertion.
Adds a module-level ``_OPS`` dict mapping name → ``operator``
callable so the parameter is the readable name (``"add"``,
``"div"``) while the test still calls the actual operator.
Cuts ~50 lines off ``test_legacy_violations.py`` and makes adding a
new operator a one-line change. Test IDs become e.g.
``test_same_size_different_labels_raises[v1-add]`` — slightly less
self-describing than the explicit-method names but cheap to read.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both methods had v1 and legacy logic interleaved via a ``fillna0`` closure that was identity under v1 and ``da.fillna(0)`` under legacy. Pull them apart into: - ``_add_constant`` / ``_apply_constant_op`` — two-line dispatchers. - ``*_v1`` — v1's implementation, reads as a single coherent story. - ``*_legacy`` — legacy's implementation, ``# LEGACY: remove at 1.0`` marker on each. At 1.0 the removal is mechanical: delete the ``_legacy`` methods and inline the ``_v1`` body into the dispatcher (or rename it back to the public name). Future readers don't have to mentally subtract the legacy branches to understand what v1 does. Add ``LEGACY: remove at 1.0`` marker comments at the other mixed sites in ``expressions.py`` so ``grep`` finds every place that needs touching: ``_align_constant``'s size-aware default fallback, ``to_constraint``'s auto-mask fallthrough, ``LinearExpression.isnull``'s historical AND, and the two warn-on-divergence sites in ``merge``. New ``arithmetics-design/legacy-removal.md`` is the master checklist for the 1.0 cut: every file, function, test, doc edit, and the safe order to do them in. The intent is that the eventual legacy removal takes an afternoon, not a week of grep-archaeology. No behaviour change — same checks, same warns, same raises. Suite is 7282 passed, 0 failures under both semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two distinct CI failures both rooted in the v1 harness commit: 1. **Test collection crash on every linopy/*.py module.** ``test/conftest.py`` imported ``linopy.config`` at module top, which loaded linopy from site-packages before pytest's ``--doctest-modules`` collection walked the source tree. The resulting __file__ mismatch broke all 22 module collections. ``pyproject.toml`` already documents this exact failure mode in the ``filterwarnings`` block. Fix: keep the constant *values* (``"legacy"`` / ``"v1"``) inline in conftest as ``_LEGACY_SEMANTICS`` etc. so the parametrize decorator doesn't force an import, and defer the ``LinopySemanticsWarning`` / ``options`` import into the fixture body. The original import comment in pyproject is now mirrored at the top of conftest. 2. **mypy: 72 "no-untyped-def" errors in test_legacy_violations.py.** The new tests were missing parameter type annotations on the fixture-injected params (``x``, ``xs``, ``op``, ``unsilenced``, ``subset``, ``A``, ``da_aux_B``, ...). ``disallow_untyped_defs`` is set globally, so test files need them too. Filled in the types (``Variable``, ``str``, ``None``, ``xr.DataArray``, ``pd.Index``), added an ``isinstance(result, LinearExpression)`` narrowing in ``test_variable_fillna_zero_revives_slot_as_present_zero`` so mypy can pick the right branch of ``fillna``'s return union. Local: 7282 passed, 0 failures under both semantics; ``mypy .`` Success. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three v1 raises were under-informative — naming the rule violated but not the operand, dim, or values involved. Make each message carry the information the helper already has: - **§5 user-NaN**: the old message conflated the two intents the user might have had — *data error* (fix with ``.fillna(value)``) vs *intended absence* (mark on the variable with ``mask=`` / ``.where`` / ``.reindex`` / ``.shift``). The new message separates them and points each to its own remedy. - **§8 merge mismatch**: rename ``merge_shared_user_coords_differ`` (bool) to ``merge_shared_user_coord_mismatch`` (tuple ``(dim, left, right) | None``). Raise text now includes the offending dim name and both sides' labels (truncated), plus the full set of resolution paths from §10: ``.sel`` / ``.reindex`` / ``.assign_coords`` / ``linopy.align`` / ``join=`` on ``.add`` / ``.sub`` / ``.mul`` / ``.div`` / ``.le`` / ``.ge`` / ``.eq``. - **§11 aux-coord conflict**: ``conflicting_aux_coord`` returns ``(name, left_vals, right_vals) | None``. Raise text includes the coord name, both value snippets, and all three resolution paths (``.drop_vars`` / ``.assign_coords`` / ``isel(drop=True)`` — ``.assign_coords`` was previously omitted). The text is now centralized in ``semantics.py`` so the two raise sites in ``expressions.py`` (``_align_constant`` and ``merge``) share one voice instead of paraphrasing each other. New ``TestErrorMessageContent`` pins the rich content in three tests — that the §5 message names both intents, that the §8 message names the dim and both label lists, and that the §11 message names the coord, both value lists, and lists all three §11 fixes (the ``.assign_coords`` omission would have slipped through ``match= "Auxiliary coordinate"`` substrings). Section references (``§5``, ``§8``, ``§11``) deliberately omitted from user-visible text — spec jargon, not a navigation aid for downstream callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the small-but-real holes in the §1–§13 coverage map. New tests
mostly, plus one code fix that the test surfaced.
§4 — absence creation
- test_where_creates_absence: §4 names ``.where(cond)`` but only
``mask=`` / ``.reindex`` were tested.
- test_unstack_creates_absence_at_missing_combinations: the
non-rectangular MultiIndex case (``stack`` preserves, ``unstack``
fills) is the asymmetry that earns its own test. Hit a real bug
on the way — ``Variable.unstack`` was producing float NaN in the
integer ``labels`` field instead of the ``FILL_VALUE`` sentinel
(-1), violating §2. Fixed by passing ``fill_value=_fill_value``
to the underlying ``Dataset.unstack`` (same pattern as ``shift``).
Audited the rest of the varwrap calls — only ``shift`` and
``unstack`` introduce new positions; the others either preserve
shape (``assign_*``, ``rename``, ``swap_dims``, ``set_index``,
``roll``, ``stack``), select existing positions (``sel`` /
``isel`` / ``drop_*``), or broadcast existing data without fill
(``broadcast_like``, ``expand_dims``).
- test_data_preserving_methods_do_not_create_absence: parameterized
over ``.roll`` / ``.sel`` / ``.isel``, regression-guards §4's
explicit contrast against the creators.
§10 — named-method join= argument
- test_add_join_override_aligns_positionally: positional-mode is the
surprising one in the join= set; pin it explicitly.
- test_reindex_like_resolves_mismatch_before_bare_op and
test_assign_coords_resolves_mismatch_before_bare_op: §10 names
these as the canonical user fixes; pin that the post-fix bare
operator actually accepts the once-mismatched operand.
§11 — auxiliary-coordinate conflicts
- test_assign_coords_resolves_conflict: §11 lists three escape
hatches; only ``.drop_vars`` / ``isel(drop=True)`` were tested.
- test_multi_operand_merge_aux_conflict_raises: the merge-path
check inspects all operands; a 3-way ``v + w + u`` with the
third disagreeing exercises that.
§12 — constraints follow the same rules
- Parameterize the existing subset / NaN / absence-propagation
tests in ``TestConstraintRHS`` over the three signs (``le`` /
``ge`` / ``eq``) via a new module-level ``_SIGNS`` dispatch.
Folds the previous ``<=`` and ``==`` duplicates together and
fills in ``>=`` for each rule (which was the explicit gap).
The PyPSA #1683 test stays separate — it's tied to ``>=`` by
the real-world case it documents.
Suite: 7303 passed, 515 skipped, 0 failures under both semantics.
``mypy .`` clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A per-op test that under-asserts (e.g. checks only .indexes) can pass under both semantics while the result silently diverges — that is how the reordered-merge mispairing slipped through. Add a parametrized guard that builds each mode-invariant operation under BOTH semantics and compares with linopy.testing's strict structural helpers (assert_linequal / assert_quadequal / assert_conequal). A regression that makes one of these paths semantics-dependent now fails loudly. Verified it catches the reordered-merge divergence as a negative control. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
Note Generated with Claude Code. Quick triage from the PyPSA-Eur side. CI red ( On the remaining checklist:
Test cleanup. The legacy-side pinning point matters most: divergent results (not just raises/warns) are the silent-failure class. Reductions over absent is the obvious one; worth grepping for other §13 / §8 sites where both code paths run clean but disagree. Will rebase the PyPSA-Eur warmstart branch on top once #744 is resolved so we can give the convention real-model exposure before rc. |
…vention # Conflicts: # linopy/alignment.py # test/test_constraint.py
Collaborator
Author
|
Performance regressions are expected. Im working on making them visible so we can adress as much as possible before merging! |
…pers After merging master, the Constraint.rhs setter accepts a DataArray, so the `# type: ignore` on the MI-level rhs tests is unused (mypy --warn-unused-ignores). Also cast the legacy-violation `_op_*` helpers to their documented runtime type, which the paired assert_linequal/assert_quadequal already verify, so `mypy .` is clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Added a Semantics report workflow (
🤖 Comment drafted by Claude (AI). |
FBumann
force-pushed
the
feat/arithmetic-convention
branch
from
c4d6bb7 to
068c106
Compare
Merging this PR will not alter performance
Comparing 
Footnotes
|
Build cost — v1 vs legacyv1 build peak & time relative to legacy, on this commit — not a comparison against master (that is CodSpeed).
Full table (time + peak, mean)📊 Interactive plots + CSV: download the semantics-report-v1-vs-legacy artifact from this run. Report-only · not a gate · refreshed on every push · obsolete once legacy is dropped. |
FBumann
force-pushed
the
feat/arithmetic-convention
branch
2 times, most recently
from
4efdff4 to
984c4b3
Compare
Add a transitional "Semantics report" workflow: it builds every benchmark spec twice — the default semantics (legacy) and LINOPY_BENCH_SEMANTICS=v1 — under the same node ids, then posts a v1-vs-legacy A/B to the PR: side-by-side peak & time relative-change plots (SVG, hosted on an auto-created assets branch) with the full text table collapsed below, plus an artifact (interactive HTML plots + CSV + text table). Report-only, this-commit only — the gate and the vs-master history stay with CodSpeed, whose ids are untouched (legacy is the unsuffixed default). Run benchmark-smoke under both conventions too; that surfaced specs that only built under legacy, fixed here: - expression_arithmetic / milp: label the square coeff arrays so v1 doesn't have to pair their equal-length axes by size (legacy guessed) - pypsa_carbon_management: skip under v1 — PyPSA emits NaN-valued constants v1 rejects by design (keys on linopy.options['semantics']) Pins pytest-benchmem[plot-static]==0.4.7 (benchmem compare/plot + kaleido SVG; clearer plot labels). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FBumann
force-pushed
the
feat/arithmetic-convention
branch
from
984c4b3 to
51841fe
Compare
MaykThewessen
left a comment
MaykThewessen
left a comment
Contributor
There was a problem hiding this comment.
One small spec-vs-code follow-up on §10 (the override / assign_coords wording), inline. Implementation already handles the size-mismatch case I'd flagged earlier; this just realigns the spec text. Not a blocker.
Comment on lines
+163
to
+170
| - The named methods — `.add` `.sub` `.mul` `.div` `.le` `.ge` `.eq` — take a | ||
| `join=` argument: `exact`, `inner`, `outer`, `left`, `right`, or `override`. | ||
| `override` is the old positional behavior — still available, but now opt-in | ||
| and named rather than triggered by a size coincidence. | ||
| - `.reindex()` / `.reindex_like()` conform an operand to a target index | ||
| (extending past the original creates absent positions — §4). | ||
| - `.assign_coords()` relabels an operand outright (positional alignment, made | ||
| explicit). |
Contributor
There was a problem hiding this comment.
§10 wording now lags the implementation. _align_constant gates join="override" on a shared-dim size check and raises before the assign_coords (expressions.py:708-733, with a comment citing §10), and the operand path routes override through xr.concat(..., join="override"), which already requires equal sizes. So the silent-broadcast hazard this paragraph reads as permitting is actually closed in code; only the spec text still describes override / .assign_coords() as unguarded positional relabels. Small clarification so the spec matches:
Suggested change
| - The named methods — `.add` `.sub` `.mul` `.div` `.le` `.ge` `.eq` — take a | |
| `join=` argument: `exact`, `inner`, `outer`, `left`, `right`, or `override`. | |
| `override` is the old positional behavior — still available, but now opt-in | |
| and named rather than triggered by a size coincidence. | |
| - `.reindex()` / `.reindex_like()` conform an operand to a target index | |
| (extending past the original creates absent positions — §4). | |
| - `.assign_coords()` relabels an operand outright (positional alignment, made | |
| explicit). | |
| - The named methods — `.add` `.sub` `.mul` `.div` `.le` `.ge` `.eq` — take a | |
| `join=` argument: `exact`, `inner`, `outer`, `left`, `right`, or `override`. | |
| `override` is the old positional behavior — still available, but now opt-in | |
| and named rather than triggered by a size coincidence. It still requires the | |
| shared dimensions to match in size: a genuine size mismatch raises rather | |
| than relabelling mismatched data, so reach for a label join (`inner` / | |
| `outer` / `left` / `right`) when the sizes really differ. | |
| - `.reindex()` / `.reindex_like()` conform an operand to a target index | |
| (extending past the original creates absent positions — §4). | |
| - `.assign_coords()` relabels an operand outright. Unlike `join="override"`, | |
| which checks the shared dims match in size, this is an *unguarded* relabel: | |
| it renames positions whether or not they correspond, so the "made explicit" | |
| here is the caller's responsibility, not a safety check. |
The line worth keeping crisp: join="override" is guarded positional alignment (size-checked, raises on mismatch); the bare .assign_coords() primitive is an unguarded relabel. Follows up the open _align_constant override thread.
Commit the v1-rollout checklist (previously an untracked scratch file) as the shared per-stage status tracker alongside goals.md/convention.md. Organised by goals.md's three stages (opt-in / default / 1.0); #744 (MultiIndex storage) is the one open design decision, gating the default flip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FBumann
force-pushed
the
feat/arithmetic-convention
branch
from
b5d5a70 to
71bd655
Compare
…703 §13's skip-absent principle is live v1 semantics for the reductions that exist (`sum`, `groupby.sum`, the objective total), so it stays in the convention. `mean` / `resample` / `coarsen` are not in linopy yet — specifying their exact semantics here is spec-ahead-of-code, so trim them to a forward-pointer to #703 (xarray method coverage), where they land as additive features that follow the same skip-absent rule. Keeps the convention == shipped behaviour. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mean/resample/coarsen are added under v1 only — legacy is a frozen compatibility layer removed at 1.0, so it never gains new operations (and needs no fill-0 semantics designed for them). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…const routing) (#804) * perf(v1): skip no-op exact-join reindex in constant ops _apply_constant_op_v1 / _add_constant_v1 resolve join=None to "exact", then _align_constant returned needs_data_reindex=True unconditionally — so a plain `coeffs * expr` ran xr.align + self.data.reindex_like (two full-dataset deepcopies) even though the factor was already broadcast to self.coords, making the exact-join alignment a no-op. first_mismatched_dim is the §8 exact check (order/label-strict via indexes.equals), so when it finds no mismatch the align changes nothing: return needs_data_reindex=False and take the cheap assign() path, skipping both copies. v1 build peak: isolated multiply -56%; full-build v1/legacy median 1.20x -> 1.00x (legacy unchanged — its default path already returned False). Full suite: 7629 passed under both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf: fold Variable*constant into one construction step (both modes) 1a3f2be rerouted Variable.__mul__(DataArray) through `to_linexpr() * other` so the §5/§8 checks fire — but that materialises a unit `1*var` expression and then multiplies it, doubling the peak on multiply-heavy builds in BOTH semantics (kvl_cycles 129→168 MB, sparse_network, masked, sos, knapsack — the CodSpeed regressions on #717). Move the checks into the one-step path instead: to_linexpr(other) now enforces §8 (mismatched coefficient coords raise v1 / warn legacy), §11 (aux-coord conflict), alongside the existing §5 NaN check, so Variable.__mul__ can fold the coefficient in at construction with no intermediate expression. Build peak vs merge-base master (1dbde37): kvl_cycles 168→129 MB (parity) in both modes; all varying-data specs 1.00×; legacy/master and v1/master medians 1.00×. Full suite 7629 passed; §5/§8/§11 alignment tests green under both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…guarded The spec text lagged the code: _align_constant size-guards join="override" (expressions.py:708-733, raises on a shared-dim size mismatch) and the operand path routes it through xr.concat(join="override") which also requires equal sizes — so override is guarded positional alignment, not an unguarded relabel. Clarify that, and distinguish it from the bare .assign_coords() primitive, which is an unguarded relabel. Per review on #717. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…744) (#803) * test: MultiIndex -> flat+aux feasibility checks (#744) Prove a first-class pd.MultiIndex snapshot can be replaced by a flat dim + level aux coords without changing the model, by explicit equality checks: reset_index normalization, where/groupby selection, per-period roll (#751), group-by-level (#751), an equivalent solved LP, and the output re-stack round-trip (flat solution -> MI via the caller's own mapping). Pins that Variable.sel does not forward MI tuple-select (#752 §2). Both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): MultiIndex -> flat+aux feasibility matrix (#744) Discussion artifact for the v1 MultiIndex decision: feasible (verified) vs desirable (opinion) matrix per op, with PyPSA call sites pinned at v1.2.4; the snapshot dim-coordinate sub-decision; the conclusion that linopy drops MultiIndex from its mental model (flat in, flat out, output re-stack owned by PyPSA); and the open PyPSA-side rows (stochastic 2nd MI, n.snapshots). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): storage-SOC LP-file equivalence; refine matrix (#744) - add test_storage_soc_lp_equivalent: the cyclic-per-period SOC "previous" built via groupby("period").roll vs an explicit positional roll produces a byte-identical LP file; a period-unaware global roll is the negative control (diverges at the period-start rows), proving the check has teeth - matrix: drop the non-operation "solve LP" row (the end-to-end compose proof is noted inline on the tracked-tests line instead); mark storage SOC 🟢 now that it is pinned by a test Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): cover non-cyclic SOC boundary (ramps too) (#744) Parametrize test_storage_soc_lp_equivalent over boundary in {cyclic, non-cyclic}. Non-cyclic (period start carries nothing) is the boundary ramps and non-cyclic storage use -- groupby("period").roll + a period-start where-mask vs an explicit positional roll, byte-identical LP either way. Teeth: the two boundaries yield different LPs (mask/wrap is not a no-op); the period-unaware global-roll control stays cyclic-only since non-cyclic masks away the rows it diverges on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): separate ramp (row-drop) from non-cyclic SOC (where) (#744) The prior "ramps too" conflated two distinct period-start boundaries, now both covered as a third parametrize value: - non-cyclic storage: drop the previous *term*, keep the row (initial SOC -> rhs). This is exactly PyPSA's `previous_soc...roll(1).where(include_previous_soc)` (constraints.py L1691) -- a `where` on the term, verified byte-faithful. - ramp: drop the whole *row* at the period start via a constraint `mask=` (PyPSA's `_period_start_mask`, L838) -- no previous to ramp from. flat (groupby roll) == oracle (positional roll) byte-for-byte in all three; each boundary differs from the cyclic baseline (teeth); global-roll control stays cyclic-only since the other two mask away the rows it diverges on. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): rename to test_period_boundary_lp_identical; show 3 boundaries (#744) The test's scope is the per-period roll's period-start boundary (cyclic / non-cyclic / ramp), not storage SOC specifically -- and it proves a byte-IDENTICAL LP file, distinct from the solve-equivalence test_per_period_lp_equivalent. Rename accordingly. Surface all three boundary spellings in the storage-SOC matrix row's flat+aux cell (wrap / .where term-drop / mask= row-drop). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): stochastic is N-D, not a second MI (#744) PyPSA's stochastic path keeps `scenario` and `name` as separate xarray dims (components/array.py: expand_dims(scenario=...), isel(scenario=0)); the `(scenario, name)` MultiIndex is manufactured only by `.stack(combined=...)` at the pandas output boundary, and common.py's MI is the input DataFrame's columns. So it is not an in-model MI like the snapshot `(period, timestep)` index -- it is already the flat+aux N-D form, nothing to decompose. Correct the stochastic row's verdict and the conclusion/decision-record prose: the one real open item is the public `n.snapshots` choice; stochastic drops from "genuine open risk / second MI" to a low-risk watch on whether #1484 ever introduces an in-model stacked index (v1.2.4 does not). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): tag boundary / non-linopy-MI rows with ⊥ (#744) Six rows are the genuine in-linopy MultiIndex decision (snapshot (period, timestep) sitting inside linopy variables). The other three are not, and now carry a ⊥ tag + legend: output (PyPSA-side boundary re-stack), n.snapshots (PyPSA public API, never a linopy model index), stochastic (not an MI at all — N-D dims). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): lead with the MI-scope premise; add reset_index root cause (#744) Open with a bold, falsifiable claim: inside the linopy model PyPSA uses a MultiIndex in exactly one place -- the snapshot dim (period, timestep). The lookalikes are ruled out inline (stochastic = N-D dims, n.snapshots = public API, output = pandas .stack), so the whole linopy-side question collapses to one axis; other axes are assumed N-D, not audited -- corrections welcome. Add a "What reset_index changes (and doesn't)" section stating the root cause: snapshot is one dim whose index is a MultiIndex, period/timestep are levels (never dims, surfacing only via .sel-collapse), and reset_index flips only the index type -- not the dim count -- turning collapse/loop/.data into direct groupby/where. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): state the snapshot-only MI scope plainly (#744) Drop the "working assumption / corrections welcome" framing -- just state that inside the linopy model MultiIndex is used only on the snapshot dim, with the three lookalikes ruled out inline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): output is in-linopy MI (snapshot out), not ⊥ boundary (#744) solution/dual carry the snapshot dim, which today is the MultiIndex inherited from the MI-indexed variables -- so output holds the same in-model snapshot MI as entry, on the way out. Drop the ⊥ tag; show the snapshot MI flowing in (entry) -> ops -> out (output) in the lead and legend. Only stochastic (N-D) and n.snapshots (public API) remain ⊥. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(mi): pin snapshots-param flat rebuild; matrix 🔵→🟢 (#744) PyPSA parks the snapshot index on model.parameters (assign(snapshots=sns), optimize.py L689) and reads it back via .snapshots.to_index() (L905) -- today that lands a real MultiIndex *inside* the linopy object. test_snapshots_param_flat_rebuild shows the flat+aux form parks only a flat snapshot + period/timestep aux vars (no MI index inside parameters) and rebuilds the identical index on demand. Flips the last in-linopy 🔵 row to 🟢. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): sink the ⊥ boundary rows to the bottom (#744) Order the matrix as the seven in-linopy snapshot-MI rows first (entry → ops → storage SOC → output → snapshots param), then the two ⊥ rows that are outside linopy (stochastic, n.snapshots) last. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): split PyPSA-side MI usages into their own table (#744) The feasibility matrix is now exactly the seven in-linopy snapshot-MI rows (all 🟢). The two ⊥ rows move to a dedicated section, "Observed PyPSA MultiIndex usages — not linopy's to solve": stochastic and n.snapshots are observed PyPSA usages that provably need no in-linopy solution and are cheap for PyPSA to handle at its boundary. They fail the in-linopy-MI test on opposite axes -- stochastic is inside linopy but not an MI; n.snapshots is an MI but not inside linopy -- which the new table states directly. Drop the now-unused ⊥ tag/legend. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): BLUF-first rewrite; fix glyph overload; condense (#744) Three structural fixes from review, one coherent pass: - Lead with a Verdict (BLUF): feasible with zero linopy changes, flat in/flat out, one open PyPSA-side item (n.snapshots). Everything after is evidence for a claim the reader already holds, instead of arguing up to it. - Fix the per-column glyph overload: feasibility uses glyphs (✅ tested / 🔲 achievable / ❌ no), desirable uses plain words (better / parity / worse) -- a glyph no longer means two things across columns. - Hoist the settled-vs-open framing above the matrix (Scope + the two-table split already embody it), drop the now-redundant decoder paragraph and restated conclusion. Net 211 → 160 lines; same evidence and call sites, less argument. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): correct "zero changes" → net subtraction; add strip analysis (#744) "Zero linopy changes" overclaimed. Feasibility needs no new capability, but adopting flat+aux is a net simplification, not a no-op: a few small additive things (positional snapshot alignment; shrink MI-input to accept-then-reset_index) against a large deletion of first-class-MI machinery. Add a "What linopy can strip" section quantifying the deletion from a read-only code sweep: ~300 lines, ~half of it one cluster -- the alignment.py level-projection subsystem (_project_onto_multiindex_levels / _enforce_implicit_projections / _LevelProjection + the projections plumbing through broadcast_to_coords), which exists solely to make a single MI level align like a dimension and is dead under flat+aux; plus ~50 lines of netcdf MI flatten/reconstruct. Notes what stays (assign_multiindex_safe, internal _term/groupby stacking, semantics §11 aux-conflict). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): frame the verdict as four wins, not just code-strip (#744) The case for dropping first-class MI isn't only "~300 fewer lines". Reframe the Verdict around four axes, each backed by a section below: - simpler implementation (What linopy can strip) - simpler mental model (What reset_index changes) - enables features (level groupby works; removes the #752 internals coupling) - no UX cost (MI still accepted in / re-stacked out; only level-tuple .sel lost) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): frame the mental-model win as linopy's own (#744) The simpler mental model is linopy's: it stops having to know pd.MultiIndex exists or cover its quirks (level alignment, index-corruption guards, reconstruction) and becomes purely xarray-native -- plain dims + aux coords, what it's already built on. MI lives only at the boundary as input/output sugar, never inside linopy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): rewrite Verdict in the maintainer's voice (#744) Lead with the linopy maintainer's actual question -- can the library be simplified without destabilising PyPSA, its primary consumer -- and answer it: PyPSA's interface doesn't move (MI in, MI-indexed out as boundary sugar), every in-linopy use has a tested identical-model flat+aux form, so nothing PyPSA relies on breaks. Stability is the frame; the wins (less to maintain, xarray-native internals, unblocks work) follow from it, and the one open item is PyPSA's own decoupled n.snapshots call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): attribute cases to PyPSA; state the generalization assumption (#744) The rows are PyPSA's MI uses *of* linopy, not linopy's own -- reword accordingly. And make the epistemics explicit: PyPSA is the one consumer audited, so these are a proof set, not a proof of universality. The argument still generalises (after reset_index everything is ordinary xarray linopy already handles; the only MI-specific capability lost for any user is level-tuple .sel), but we now state the assumption that no other user has an infeasible MI case, and invite counterexamples. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): dense rewrite — Decision + Appendix shape (#744) Restructure to one decision screen + appendix (the chosen shape), 216 → 141 lines: - Verdict: claim + 3 wins + open item + proof-set caveat, tightened. - The evidence: both tables (in-model matrix, boundary usages) under one heading; the three post-matrix findings paragraphs collapse to one caption; Scope and the side-finding fold in. - The payoff: strip table + a line; "stays"/"additive" compressed. - Appendix: the three overlapping mechanics sections merged/condensed (reset_index + data-model table, dim-coordinate sub-decision, transition shape) + decision record. Each claim stated once; explanatory prose turned into captions. Tables and all call-site links preserved verbatim; only the #756/#757 links in a memory-cost aside were trimmed (the point survives without them). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): fix "MI-indexed results back" — linopy returns flat (#744) linopy does not hand MI-indexed results back; that contradicts flat-in/flat-out. It returns a flat solution; PyPSA re-applies its own n.snapshots index when mapping the result onto components (assign_solution already does this), so PyPSA's results stay MI-indexed -- but the re-stack is PyPSA's, not linopy's. MI is boundary sugar PyPSA owns, never inside linopy's model. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): resolve "no change" vs "input sugar" contradiction (#744) Separate feasibility from the entry policy. Feasibility needs no new linopy capability (the rewrites use existing ops; entry is a plain reset_index). Whether linopy accepts an MI and flattens it (input sugar, backwards-compatible) or rejects MI so callers flatten first (purest, breaks existing callers) is a boundary policy, now surfaced as an explicit Decision-record item with a recommendation (accept-as-sugar, optional deprecation path to flat-only) -- not smuggled in as both "no change" and "accept sugar" at once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): consolidate into a Design decisions table (#744) Replace the prose "decision record" + scattered "small additive work" mentions with one top-level Design decisions table: internal model, MI-on-input, snapshot alignment, output, n.snapshots -- each with options and a recommendation/status. Also fold in the input-policy correctness fix: MI in / flat out is a silent contract change, so silent auto-flatten is ruled out -- reject, or accept-and-warn (undecided). Slim the Verdict to point at the table; only n.snapshots is flagged as the substantive open item, and it's PyPSA's call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): MI surface is bigger — two-layer payoff + corrected sweep (#744) The ~300 estimate was a floor. Two corrections from a deeper measure: - assign_multiindex_safe (the #303 "would corrupt the index" workaround, 39 call sites) is snapshot-MI-driven, not a general helper: internal stacks use create_index=False, so the snapshot MI is the only thing that triggers the corruption warning. Under flat+aux all 39 sites revert to plain .assign(). The first sweep wrongly marked it "stays" -- corrected, with a strip-table row. - Add the diffuse "cognitive tax" layer the line-count misses: 6 isinstance(MI) guards, 17 level-ops, ~40 quirk-comments, and 169 lines of MI edge-case tests across 10 files. Reframe the mental-model pillar around that defensive surface (guards, the 39-site corruption workaround, quirk-comments, edge-case tests) rather than just lines. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): add latent-risk and MI-not-promoted arguments (#744) Two more points strengthening the case: - Latent risk: MI's edge surface almost certainly isn't fully covered (the #303 corruption is one gap that already bit), so keeping MI carries unknown-bug risk that flat+aux retires. Folded into the mental-model pillar and the payoff. - MI isn't a promoted feature, so few users lean on it -- lowers the cost of the lost level-tuple .sel and makes the "no other infeasible case" generalization safer. Folded into the proof-set caveat. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): add the second MI surface — multi-key groupby (#744) The "MI in exactly one place — snapshot" claim was false. Verified here (both semantics): a multi-key groupby (DataFrame grouper) mints a stacked `group` MultiIndex, and PyPSA-Eur's add_CCL_constraints consumes it richly (groupby(country×carrier).sum() then .intersection / .loc[MI] / .sel(group=index), solve_network.py L1064/L1080-1095). add_EQ/add_BAU are single-key (no group MI) -- correcting an over-broad relayed claim. Integrate at "2nd open item, flagged harder": scope the Verdict's snapshot claim, turn one open item into two (n.snapshots + multi-key groupby), add a "Second MI surface" section and a Design-decisions row, and make the proof-set caveat honest (the generalization already cracked once). Framing per FBumann: it's the same flat+aux change, linopy-owned (groupby returns flat group + key aux coords) -- but a public-API change with external consumers, so it needs a deprecation path, not a boundary fix. Blast radius (-Earth/-DE/plugins) not yet scoped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): fold multi-key groupby into the matrix as the 🔲 row (#744) Per FBumann: now that the deprecation is just the standard legacy/v1 path (warn under legacy, raise/flat+aux under v1), multi-key groupby is the same kind of in-linopy MI as the snapshot rows -- siloing it overstated its difference. Move it into table 1 as the 8th row, marked 🔲 (the one achievable-not-yet-tested row, vs the 7 tested ✅ -- which also finally exercises the glyph legend). Replace the standalone "Second MI surface" section with a tight caption (verified both semantics; PyPSA-Eur CCL cite; legacy/v1 migration; blast radius the only open part). Soften the earlier "harder / needs a deprecation path" framing throughout: it reaches external code but rides the standard switch, so it's guided, not a hard break. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(v1): multi-key groupby returns a flat group dim under v1 (#744) A frame grouper (`groupby(DataFrame).sum()`, e.g. PyPSA-Eur's CCL country×carrier) mints a stacked `group` MultiIndex today. Under v1 it now returns a flat `group` dim with the keys as ordinary aux coords — uniform with every other dim, no MI; select via groupby/where, not a `.sel(group=tuple)` level tuple. Legacy keeps the stacked MultiIndex and emits a DeprecationWarning pointing at the change. Scope: only the cases where a stacked group MI survives today — the DataFrame grouper and `observed=True`. The default `observed=False` list-of-names path that already unstacks into separate dims is untouched. The legacy deprecation warning is limited to the user-facing frame grouper. This is the first transition slice for dropping first-class MultiIndex in v1 (the strip of the now-dead-under-v1 MI machinery follows at legacy EOL). Affected groupby tests split into @pytest.mark.legacy (MI + asserts the warning) / @pytest.mark.v1 (flat+aux) twins. Full suite green under both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: make the MI `u` fixture semantics-aware for the v1 MI drop (#744) Toward v1 disallowing MultiIndex entirely: the shared `u` fixture now yields a (level1, level2) MultiIndex variable under legacy and the equivalent flat `dim_3` dim + level1/level2 aux coords under v1. Moved it out of the central `m` fixture so `m` is MI-free. The two v1 tests that asserted MI-specific behavior are rewritten to the flat+aux idiom: the level-projection "raises" test becomes explicit per-level weighting via the aux coord; the MI inner-join alignment test is marked legacy (flat-dim alignment is covered by the other align tests). Prep only — MI is still allowed under v1 at this point; no v1 test relies on it now. Suite green under both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(v1): disallow MultiIndex dim-coords entirely (#744) v1 now rejects a pandas MultiIndex dimension anywhere it would enter the model; legacy keeps it with a DeprecationWarning. This supersedes the branch's earlier "MI-allowed-but-strict" v1 design — under v1 a snapshot (or any dim) must be a flat dim with the levels as auxiliary coords (`reset_index`). enforce_no_multiindex(obj, *, context) scans an object's dim-indexes for a MultiIndex and raises (v1) / warns (legacy) with a message naming what the user did. It's wired at the caller-facing boundaries where MI can enter a persistent object, so the error is precise: - add_variables → context="variable 'x'" (catches coords and DataArray bounds) - add_constraints → context="constraint 'c'" (catches MI introduced via arithmetic) Tests: MultiIndex is now a legacy-only concept, so MI-building tests are marked legacy (or their fixtures skip under v1), and the obsolete @v1 tests that asserted the old MI-under-v1 semantics are removed and replaced by a v1 reject test. Full suite green under both semantics. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(v1): guard add_objective against MultiIndex; hoist imports (#744) add_objective assigns the expression directly (bypassing Objective.__init__), so add the enforce_no_multiindex check there too — an MI forced onto an expression and handed to the objective now rejects under v1 with the "objective ..." context. Covered by a v1 reject test alongside the add_variables/add_constraints ones. Also move the `enforce_no_multiindex` import to module level in model.py (no import cycle — semantics imports none of model/variables/constraints/objective). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(arithmetics): track open-items.md; mark #744 resolved (#744) Commit the v1-rollout checklist (previously untracked) as the shared status tracker, refreshed to the current state: the one open design decision (#744, MultiIndex storage) is resolved — v1 disallows MI — and the remaining work is organised by goals.md's three stages (opt-in / default / 1.0). The transition-surface-complete + changelog items are placed at stage 1, matching goals.md step 1 (warn on legacy, raise on v1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(v1): repr shows flattened MultiIndex as its level coords A flat dim carrying decomposed-MI levels as aux coords now renders like a first-class MultiIndex — header 'dim (l0, l1)', rows '[(a, 1)]', var refs 'x[(a, 1)]' (incl. explicit LP/MPS export names). Shared helpers in common.py: index_level_coords / dim_level_names / label_coord / get_printout_labels. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Fabian <fab.hof@gmx.de> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.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
3 participants
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.
The strict v1 semantic convention for linopy — predictable coordinate alignment and NaN handling.
This is the master PR for the new semantic convention in linopy. It starts with our
Design & transitioning goals, which is carried out in ourNew Semantics spec. Both files are tracked in this branch. WHat you read is the current state.Both might change until this PR is merged
The concrete rollout checklist — the three stages (opt-in → default → 1.0) and their per-stage status — is tracked in
arithmetics-design/open-items.md.Scope
The convention ships behind
linopy.options["semantics"]—v1opt-in,legacythe default. This PR carries the design, spec, tests and implementation; documentation notebooks follow separately.Testing
All tests in linopy will be executed for both semantics.
Differing behaviour will be tested using
pytest.markers.This will increase ci time temporarily until v1 is released.
Defered to a follow up PR
Docs: Documentation, Migration guide, Release notes