diff --git a/arithmetics-design/convention.md b/arithmetics-design/convention.md index 05c3f9c77..9eb743187 100644 --- a/arithmetics-design/convention.md +++ b/arithmetics-design/convention.md @@ -127,16 +127,21 @@ array is treated as a scalar; a Python `list` is read as a numpy array (it carries values, not labels). Implemented in `linopy.alignment` ([#736]). -### §8. Shared dimensions must match exactly +### §8. Shared dimensions must carry the same labels -If two operands share a dimension, their coordinate labels must be identical, -or the operator raises `ValueError`. +If two operands share a dimension, their coordinate labels must be the same +*set*, or the operator raises `ValueError`. Order is immaterial: the same +labels in a different order are the same coordinate and align by label (a +reindex), following "by label, never by position" above — only a difference in +the label set raises. -This is xarray's model with `arithmetic_join="exact"` — deliberately stricter -than xarray's own default (`inner`). An inner join silently drops the +This is close to xarray's `arithmetic_join="exact"` — deliberately stricter +than xarray's own default (`inner`) — but order-independent, where xarray's +`exact` would reject a pure reorder. An inner join silently drops the non-overlapping labels, and in an optimization model a dropped coordinate is a -dropped term or constraint: a silent wrong answer. An exact match surfaces the -mismatch where it happens. (The [pyoframe] library uses the same model.) +dropped term or constraint: a silent wrong answer. Matching on the label set +surfaces a real mismatch where it happens. (The [pyoframe] library uses the +same model.) Because the rule is identical for every operator, the operator-alignment split ([#708]) — `*` aligning by label while `+`, `-`, `/` go by position — @@ -202,7 +207,9 @@ values: An input that reconstructs the *entire* MultiIndex (all levels, every combination) is not a conflict — it is the same coordinate spelled -differently, and aligns element-wise under §8. +differently, and aligns by tuple under §8, in any order. Order is +immaterial here exactly as for a plain dimension: the same tuples in a +different order are reordered to match, not rejected. (Legacy projects implicitly and warns — scenario B of the [#732]/[#737] discussion; the implicit projection is removed at 1.0.) diff --git a/linopy/expressions.py b/linopy/expressions.py index 7ff13775a..7e46af39b 100644 --- a/linopy/expressions.py +++ b/linopy/expressions.py @@ -90,14 +90,15 @@ ) from linopy.semantics import ( _legacy_coord_mismatch_message, + _legacy_coord_reorder_message, _legacy_nan_rhs_constraint_message, _shared_dim_mismatch_message, absorb_absence, check_user_nan, + conform_merge_dims, enforce_aux_conflict, first_mismatched_dim, is_v1, - merge_shared_user_coord_mismatch, warn_legacy, ) from linopy.types import ( @@ -2659,26 +2660,24 @@ def merge( data = [e.data if isinstance(e, linopy_types) else e for e in exprs] data = [fill_missing_coords(ds, fill_helper_dims=True) for ds in data] - # §11: aux-coord conflict is independent of dim alignment — fires on - # every join path. xr.concat(..., compat="override") silently drops - # the conflicting aux coord, which is the #295 bug v1 closes; we must - # raise (v1) / warn (legacy) before xr.concat sees the data, regardless - # of how the caller resolves the §8 dim mismatch. - enforce_aux_conflict(data) - - # §8: shared *user* dimension coordinates must match exactly across all - # operands. Helper dims (_term, _factor) legitimately differ, so we - # validate user dims separately and keep xr.concat on join="outer" - # (which doesn't enforce "exact" — that's what this check is for). + # §8: v1 aligns a reorder by label and raises a dim mismatch (before the + # §11 aux check, else a MultiIndex mismatch reads as a level-coord + # conflict). Legacy keeps positional alignment and only warns. if join is None: - mismatch = merge_shared_user_coord_mismatch(data, concat_dim=dim) - if is_v1() and mismatch is not None: - raise ValueError(_shared_dim_mismatch_message(*mismatch)) - # LEGACY: remove at 1.0 — warn-on-divergence is the migration signal. - if mismatch is not None: + data, mismatch, reorder = conform_merge_dims(data, concat_dim=dim) + if is_v1(): + if mismatch is not None: + raise ValueError(_shared_dim_mismatch_message(*mismatch)) + elif mismatch is not None: # LEGACY: remove at 1.0 warn_legacy( - _legacy_coord_mismatch_message(f"merge along dim {dim!r}", *mismatch), + _legacy_coord_mismatch_message(f"merge along dim {dim!r}", *mismatch) ) + elif reorder is not None: # LEGACY: remove at 1.0 + warn_legacy( + _legacy_coord_reorder_message(f"merge along dim {dim!r}", *reorder) + ) + + enforce_aux_conflict(data) # §11 if join is not None: override = join == "override" diff --git a/linopy/semantics.py b/linopy/semantics.py index 1ef43cd58..38e066956 100644 --- a/linopy/semantics.py +++ b/linopy/semantics.py @@ -152,6 +152,19 @@ def _legacy_coord_mismatch_message( ) +def _legacy_coord_reorder_message(context: str, dim: str, left: Any, right: Any) -> str: + """Same labels, different order — aligned positionally by legacy; v1 reindexes.""" + return ( + f"Coordinate order mismatch in {context} aligned positionally by legacy." + " Under v1 the same labels in a different order align by label (a" + " reindex), giving a different result." + f"\n Dim: {dim!r}: left={_short_repr(left)}, right={_short_repr(right)}" + "\n Resolve: `.sel(...)` / `.reindex(...)` to align" + "\n `.assign_coords(...)` to relabel one side" + "\n or pass an explicit `join=` argument." + _OPT_IN_HINT + ) + + def _legacy_aux_conflict_message(name: str, left: Any, right: Any, kind: str) -> str: """ Conflicting aux coord silently dropped by xarray under legacy. @@ -299,31 +312,56 @@ def first_mismatched_dim(a: DataArray, b: DataArray) -> tuple[str, Any, Any] | N return None -def merge_shared_user_coord_mismatch( +def conform_merge_dims( datasets: Sequence[Dataset], concat_dim: str -) -> tuple[str, Any, Any] | None: +) -> tuple[list[Dataset], tuple[str, Any, Any] | None, tuple[str, Any, Any] | None]: """ - Find a shared user dim where the operands' labels disagree. - - Returns ``(dim_name, left_labels, right_labels)`` for the first - mismatch found, or ``None`` if all operands agree. Helper dims - (``_term``, ``_factor``) and the concat dim itself are excluded — - those legitimately vary across the operands being merged. Compares - bare dimension indexes (``d.indexes[k]``) so non-dim (auxiliary) - coords are ignored — those are §11's job. + Inspect shared user dims for a merge, in a single pass over the operands. + + Returns ``(data, mismatch, reorder)``. A shared user dim whose labels are + the first operand's in a different order (same set, including a stacked + MultiIndex's tuples) is a *reorder*; one whose label set differs is a + *mismatch* — each reported as ``(dim, first_labels, other_labels)`` (first + found). Under v1, reorders are aligned to the first operand's order in the + returned data (via positional ``isel`` — ``reindex`` cannot reorder a + MultiIndex by tuple) and ``reorder`` is ``None``; the caller raises on + ``mismatch``. Under legacy, nothing is aligned and the caller warns: + ``reorder`` (v1 would align by label) or ``mismatch`` (v1 would raise). + Helper dims (``_term``, ``_factor``) and the concat dim are excluded; bare + dimension indexes are compared, so auxiliary coords stay §11's job. """ + datasets = list(datasets) + if len(datasets) < 2: + return datasets, None, None skip = set(HELPER_DIMS) | {concat_dim} - per_ds = [ + indexed = [ {k: d.indexes[k] for k in d.dims if k not in skip and k in d.indexes} for d in datasets ] - shared = set.intersection(*(set(p.keys()) for p in per_ds)) if per_ds else set() - for d_name in shared: - ref = per_ds[0][d_name] - for p in per_ds[1:]: - if not ref.equals(p[d_name]): - return str(d_name), ref.values, p[d_name].values - return None + shared = set.intersection(*(set(p) for p in indexed)) + if not shared: + return datasets, None, None + + permute = is_v1() + out = [datasets[0]] + mismatch: tuple[str, Any, Any] | None = None + reorder: tuple[str, Any, Any] | None = None + for i in range(1, len(datasets)): + plan: dict[Any, Any] = {} + for d in shared: + ref, idx = indexed[0][d], indexed[i][d] + if ref.equals(idx): + continue + positions = idx.get_indexer(ref) if len(idx) == len(ref) else None + if positions is not None and (positions >= 0).all(): + if permute: + plan[d] = positions + elif reorder is None: + reorder = (str(d), ref.values, idx.values) + elif mismatch is None: + mismatch = (str(d), ref.values, idx.values) + out.append(datasets[i].isel(plan) if plan else datasets[i]) + return out, mismatch, reorder def conflicting_aux_coord( diff --git a/test/test_legacy_violations.py b/test/test_legacy_violations.py index 5482770ac..c4f0ecebe 100644 --- a/test/test_legacy_violations.py +++ b/test/test_legacy_violations.py @@ -58,6 +58,7 @@ class corresponds to a section of ``arithmetics-design/convention.md`` from linopy import Model from linopy.config import LinopySemanticsWarning +from linopy.expressions import merge from linopy.testing import assert_linequal from linopy.variables import Variable @@ -804,6 +805,119 @@ def test_var_plus_var_broadcast_non_shared_dim_works( result = a + b assert set(result.coord_dims) == {"time", "scenario"} + @pytest.mark.v1 + def test_var_plus_var_reordered_labels_align(self, m: Model) -> None: + a = m.add_variables(coords=[pd.Index(["costs", "penalty"], name="e")], name="a") + b = m.add_variables(coords=[pd.Index(["penalty", "costs"], name="e")], name="b") + result = (1 * a) + (1 * b) + assert list(result.indexes["e"]) == ["costs", "penalty"] + + @pytest.mark.v1 + def test_reordered_constants_pair_by_label_not_position(self, m: Model) -> None: + ea = pd.Index(["costs", "penalty"], name="e") + eb = pd.Index(["penalty", "costs"], name="e") + a = m.add_variables(coords=[ea], name="a") + pd.Series([100.0, 200.0], index=ea) + b = m.add_variables(coords=[eb], name="b") + pd.Series([1.0, 2.0], index=eb) + result = a + b + assert float(result.const.sel(e="costs")) == 102.0 + assert float(result.const.sel(e="penalty")) == 201.0 + + @pytest.mark.v1 + def test_multi_operand_merge_reordered_pairs_by_label(self, m: Model) -> None: + ea = pd.Index(["x", "y", "z"], name="e") + er = pd.Index(["z", "y", "x"], name="e") + a = m.add_variables(coords=[ea], name="a") + pd.Series( + [1.0, 2.0, 3.0], index=ea + ) + b = m.add_variables(coords=[er], name="b") + pd.Series( + [10.0, 20.0, 30.0], index=er + ) + c = m.add_variables(coords=[ea], name="c") + pd.Series( + [100, 200, 300.0], index=ea + ) + result = merge([a, b, c], cls=type(a)) + assert float(result.const.sel(e="x")) == 131.0 + assert float(result.const.sel(e="z")) == 313.0 + + @pytest.mark.v1 + def test_quadratic_merge_reordered_aligns(self, m: Model) -> None: + ea = pd.Index(["x", "y", "z"], name="e") + er = pd.Index(["z", "y", "x"], name="e") + x = m.add_variables(coords=[ea], name="x") + y = m.add_variables(coords=[er], name="y") + result = (x * x) + (y * y) + assert list(result.indexes["e"]) == ["x", "y", "z"] + + @pytest.mark.v1 + def test_reordered_multiindex_aligns_by_tuple(self, m: Model) -> None: + mi1 = pd.MultiIndex.from_tuples( + [(1, "a"), (1, "b"), (2, "a")], names=["p", "s"] + ) + mi2 = pd.MultiIndex.from_tuples( + [(2, "a"), (1, "b"), (1, "a")], names=["p", "s"] + ) + mi1.name = "snap" + mi2.name = "snap" + x = m.add_variables(coords=[mi1], name="x") + pd.Series( + [1.0, 2.0, 3.0], index=mi1 + ) + y = m.add_variables(coords=[mi2], name="y") + pd.Series( + [10.0, 20.0, 30.0], index=mi2 + ) + result = x + y + got = dict(zip(map(tuple, result.indexes["snap"]), result.const.values)) + assert got == {(1, "a"): 31.0, (1, "b"): 22.0, (2, "a"): 13.0} + + @pytest.mark.legacy + def test_reordered_merge_positional_legacy(self, m: Model) -> None: + ea = pd.Index(["costs", "penalty"], name="e") + eb = pd.Index(["penalty", "costs"], name="e") + a = m.add_variables(coords=[ea], name="a") + pd.Series([100.0, 200.0], index=ea) + b = m.add_variables(coords=[eb], name="b") + pd.Series([1.0, 2.0], index=eb) + result = a + b + assert float(result.const.sel(e="costs")) == 101.0 + assert float(result.const.sel(e="penalty")) == 202.0 + + @pytest.mark.legacy + def test_reordered_merge_warns_legacy(self, m: Model, unsilenced: None) -> None: + ea = pd.Index(["costs", "penalty"], name="e") + eb = pd.Index(["penalty", "costs"], name="e") + a = m.add_variables(coords=[ea], name="a") + b = m.add_variables(coords=[eb], name="b") + with pytest.warns(LinopySemanticsWarning) as record: + (1 * a) + (1 * b) + msg = next( + str(w.message) for w in record if w.category is LinopySemanticsWarning + ) + assert msg == ( + "Coordinate order mismatch in merge along dim '_term' aligned " + "positionally by legacy. Under v1 the same labels in a different " + "order align by label (a reindex), giving a different result." + "\n Dim: 'e': left=['costs', 'penalty'], " + "right=['penalty', 'costs']" + "\n Resolve: `.sel(...)` / `.reindex(...)` to align" + "\n `.assign_coords(...)` to relabel one side" + "\n or pass an explicit `join=` argument." + "\n Opt in: linopy.options['semantics'] = 'v1'" + "\n Silence: warnings.filterwarnings('ignore', " + "category=LinopySemanticsWarning)" + ) + + @pytest.mark.v1 + def test_different_multiindex_raises_dim_mismatch(self, m: Model) -> None: + mi1 = pd.MultiIndex.from_tuples( + [(1, "a"), (1, "b"), (2, "a")], names=["p", "s"] + ) + mi2 = pd.MultiIndex.from_tuples( + [(1, "a"), (1, "b"), (3, "c")], names=["p", "s"] + ) + mi1.name = "snap" + mi2.name = "snap" + x = m.add_variables(coords=[mi1], name="x") + y = m.add_variables(coords=[mi2], name="y") + with pytest.raises(ValueError, match="shared dimension 'snap'"): + (1 * x) + (1 * y) + @pytest.mark.legacy def test_var_plus_var_different_labels_silent( self, x: Variable, x_other: Variable @@ -1534,6 +1648,17 @@ def test_var_plus_var_aux_conflict_raises(self, m: Model, A: pd.Index) -> None: with pytest.raises(ValueError, match="Auxiliary coordinate"): v + w + @pytest.mark.v1 + def test_aux_conflict_survives_reordered_dim(self, m: Model) -> None: + v = m.add_variables( + lower=0, coords=[pd.Index(["x", "y", "z"], name="A")], name="v" + ).assign_coords(B=("A", [1, 2, 3])) + w = m.add_variables( + lower=0, coords=[pd.Index(["z", "y", "x"], name="A")], name="w" + ).assign_coords(B=("A", [1, 2, 3])) + with pytest.raises(ValueError, match="Auxiliary coordinate"): + v + w + @pytest.mark.v1 def test_mul_constant_aux_conflict_raises(self, m: Model, A: pd.Index) -> None: """Same rule on the multiplication path — not just ``+``."""