From 0402431f6e6e0c34d2f67d3e5b2d53628ed80fb7 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Sun, 9 Aug 2026 16:54:13 +0200 Subject: [PATCH 1/2] fix(piecewise): declare ragged breakpoints with mask= instead of inferring NaN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ragged curves — entities with different numbers of breakpoints — are stored densely along `_breakpoint` with the surplus slots left absent. Under v1 that tripped the §5 user-NaN guard from deep inside the formulation's arithmetic, with a message pointing at remedies that do not apply to breakpoint data. All formulation paths were affected: `lp` and `sos2` and `incremental` (with and without `active=`) and disjunctive. §5 is right to refuse the input: a shorter curve and a data error look identical, and linopy trusts NaN only from its own structural operations (§4). What was missing is a way to declare the absence. Add `mask=` to `add_piecewise_formulation` — §4's mechanism, already the vocabulary of `add_variables`/`add_constraints` — and make it the authoritative breakpoint mask. Legacy keeps inferring from NaN placement, now with a LinopySemanticsWarning. Once declared, the padding still must not reach the arithmetic as a *constant*: provenance is gone by the time a breakpoint table multiplies a variable, so §5 applies to it there. Zero those coefficients — the absence is already carried by the masked variable and propagates on its own (§6). `tangent_lines` has no mask to declare with, so it can only report; it now names raggedness rather than surfacing the generic user-NaN message. Closes #884 Co-Authored-By: Claude Opus 5 (1M context) --- doc/release_notes.rst | 1 + linopy/piecewise.py | 122 ++++++++++++-- linopy/semantics.py | 46 ++++++ test/test_piecewise_constraints.py | 256 +++++++++++++++++++++++++++-- 4 files changed, 403 insertions(+), 22 deletions(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 6c65602eb..4d49f0f34 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -35,6 +35,7 @@ Upcoming Version * Default internal integer labels to ``int32``, cutting memory ~25% and speeding up model build 10-35%. Models exceeding the int32 maximum (~2.1 billion labels) widen to ``int64`` automatically with a ``UserWarning``; pass ``Model(dtypes={"labels": np.int64})`` upfront to avoid the mid-build upcast (exposed read-only via ``Model.dtypes``). * ``add_variables(binary=True, ...)`` now accepts ``lower``/``upper`` bounds, as long as they are 0 or 1. Previously binary bounds could only be set via the ``.lower``/``.upper`` setters after creation. (https://github.com/PyPSA/linopy/issues/776) +* ``add_piecewise_formulation`` gained a ``mask`` parameter declaring which breakpoint slots hold a real breakpoint. It is needed for **ragged** curves — entities with different numbers of breakpoints — which are stored densely with the surplus slots left absent. Under v1 that absence must be declared (``mask=x_pts.notnull()``) rather than read off the NaN padding, since §5 does not let linopy tell a shorter curve from a data error; legacy keeps inferring it, with a ``LinopySemanticsWarning``. Previously ragged curves failed under v1 with a generic "NaN found in a user-supplied constant" raised from deep inside the formulation's arithmetic, on all of the ``lp``, ``sos2``, ``incremental`` and disjunctive paths. (https://github.com/PyPSA/linopy/issues/884) * ``add_piecewise_formulation`` gained an ``active_fill`` parameter that gates a partial ``active`` (defined over a subset of the indexed dimension, or masked) as always-active (``1``) or always-off (``0``); without it, a partial ``active`` — which was previously zeroed silently — now raises. Useful when one formulation mixes gated and ungated entities (e.g. committable and non-committable units sharing a ``status``). ``active_fill`` is transitional and will be removed once v1 semantics make ``active.reindex(coords).fillna(value)`` sufficient. (https://github.com/PyPSA/linopy/issues/796) *Documentation* diff --git a/linopy/piecewise.py b/linopy/piecewise.py index 868409342..339ff9359 100644 --- a/linopy/piecewise.py +++ b/linopy/piecewise.py @@ -50,12 +50,13 @@ EvolvingAPIWarning, sign_replace_dict, ) +from linopy.semantics import check_user_nan_breakpoints if TYPE_CHECKING: from linopy.constraints import Constraint, Constraints from linopy.expressions import LinearExpression from linopy.model import Model - from linopy.types import LinExprLike + from linopy.types import LinExprLike, MaskLike from linopy.variables import Variables logger = logging.getLogger(__name__) @@ -736,12 +737,17 @@ def _tangent_lines_impl( x: LinExprLike, x_points: BreaksLike, y_points: BreaksLike, + piece_mask: DataArray | None = None, ) -> LinearExpression: """ Chord-expression math — the body of ``tangent_lines`` without the :class:`EvolvingAPIWarning`. Called internally by ``_add_lp`` so a single ``add_piecewise_formulation((y, y_pts, "<="), (x, x_pts))`` emits exactly one warning, not two. + + ``piece_mask`` marks the pieces that exist. A padded piece of a ragged + curve gets a neutral zero chord, which the caller is expected to mask + out of the resulting constraint. """ from linopy.expressions import LinearExpression as LinExpr from linopy.variables import Variable @@ -753,7 +759,7 @@ def _tangent_lines_impl( dy = y_points.diff(BREAKPOINT_DIM) piece_index = np.arange(dx.sizes[BREAKPOINT_DIM]) - slopes = _rename_to_pieces(dy / dx, piece_index) + slopes = _drop_absent(_rename_to_pieces(dy / dx, piece_index), piece_mask) x_base = _rename_to_pieces( x_points.isel({BREAKPOINT_DIM: slice(None, -1)}), piece_index ) @@ -761,7 +767,7 @@ def _tangent_lines_impl( y_points.isel({BREAKPOINT_DIM: slice(None, -1)}), piece_index ) - intercepts = y_base - slopes * x_base + intercepts = _drop_absent(y_base - slopes * x_base, piece_mask) if not isinstance(x, Variable | LinExpr): raise TypeError(f"x must be a Variable or LinearExpression, got {type(x)}") @@ -830,6 +836,13 @@ def tangent_lines( "entirely with " '`warnings.filterwarnings("ignore", category=linopy.EvolvingAPIWarning)`.', ) + # No mask to declare absence on this low-level helper, so a ragged curve + # can only be reported (§5), not resolved — say so here rather than let + # the generic user-NaN message surface from the chord arithmetic. + if bool(_coerce_breaks(x_points).isnull().any()) or bool( + _coerce_breaks(y_points).isnull().any() + ): + check_user_nan_breakpoints() return _tangent_lines_impl(x, x_points, y_points) @@ -983,6 +996,69 @@ def _paired_valid_points(*points: DataArray) -> DataArray: return points[0].where(~invalid) +def _drop_absent(values: DataArray, mask: DataArray | None) -> DataArray: + """ + Make a breakpoint table safe to use as a *constant* operand. + + Absent slots are marked with ``NaN`` (§2), but as soon as the table + multiplies a variable it is an ordinary constant, and §5 rejects NaN + there — provenance is gone by that point. The absence is already + carried by the variable, which was created with ``mask=`` (§4), so it + propagates on its own (§6) and the coefficient at that slot is never + read. Replace it with a neutral zero. + """ + if mask is None: + return values + return values.where(mask, 0.0) + + +def _resolve_breakpoint_mask( + bp_list: list[DataArray], mask: MaskLike | None +) -> tuple[list[DataArray], DataArray | None]: + """ + Settle which slots hold a real breakpoint, and align the values to it. + + A ragged curve is stored densely along ``BREAKPOINT_DIM`` with the + surplus slots absent, which §2 encodes as ``NaN``. §5 forbids reading + that marker off *user* data — a shorter curve and a data error are + indistinguishable — so the caller declares the absence through §4's + ``mask=`` and this returns it. Without a declaration, v1 raises and + legacy keeps inferring from NaN placement. + + The returned arrays carry ``NaN`` exactly where the mask says absent, + so ``isnull()`` stays the single predicate (§3) downstream. + """ + combined_null = bp_list[0].isnull() + for bp in bp_list[1:]: + combined_null = combined_null | bp.isnull() + + if mask is None: + if not bool(combined_null.any()): + return bp_list, None + check_user_nan_breakpoints() + return bp_list, ~combined_null + + declared = (mask if isinstance(mask, DataArray) else DataArray(mask)).astype(bool) + bp_dims = set().union(*(bp.dims for bp in bp_list)) + extra = set(declared.dims) - bp_dims + if extra: + raise ValueError( + f"`mask` is not broadcastable against the breakpoints: it has " + f"dimension(s) {sorted(map(str, extra))} which the breakpoints " + f"({sorted(map(str, bp_dims))}) do not have." + ) + _, declared = xr.broadcast(bp_list[0], declared) + + if bool((combined_null & declared).any()): + raise ValueError( + "NaN at a breakpoint slot that `mask` marks as present. Either " + "widen the mask to cover those slots or fix the values with " + "`.fillna(...)`." + ) + + return [bp.where(declared) for bp in bp_list], declared + + def _validate_shared_coords(points: Sequence[DataArray]) -> None: skip = {BREAKPOINT_DIM, SEGMENT_DIM} | set(HELPER_DIMS) for i, left in enumerate(points): @@ -1074,6 +1150,7 @@ def add_piecewise_formulation( method: PWL_METHOD = "auto", active: LinExprLike | None = None, active_fill: int | None = None, + mask: MaskLike | None = None, name: str | None = None, ) -> PiecewiseFormulation: r""" @@ -1183,6 +1260,25 @@ def add_piecewise_formulation( Transitional convenience: under v1 semantics, pad ``active`` explicitly with ``active.reindex(coords).fillna(value)`` instead — this parameter is slated for removal then. + mask : MaskLike, optional + Which breakpoint slots hold a real breakpoint — ``True`` where one + exists, ``False`` where it is absent. Shaped like the breakpoint + arrays (entity dims × ``_breakpoint``), or anything that broadcasts + against them. + + Needed only for **ragged** curves, where entities have different + numbers of breakpoints. These are stored densely with the surplus + slots left absent, and under v1 that absence has to be declared + rather than read off the data: a shorter curve and a stray NaN look + identical, so linopy refuses to guess (see the convention, §4/§5). + For breakpoints that are already NaN-padded, the declaration is + ``mask=x_pts.notnull()``. + + Slots marked absent are excluded from the formulation: no auxiliary + variable is created for them and no constraint references them. + Passing a mask that hides a *present* value is allowed and drops + that breakpoint; a NaN at a slot the mask calls present is a data + error and raises. name : str, optional Base name for generated variables/constraints. @@ -1319,10 +1415,7 @@ def add_piecewise_formulation( _validate_shared_coords(bp_list) _validate_expr_coords(bp_list, lin_exprs) - combined_null = bp_list[0].isnull() - for bp in bp_list[1:]: - combined_null = combined_null | bp.isnull() - bp_mask = ~combined_null if bool(combined_null.any()) else None + bp_list, bp_mask = _resolve_breakpoint_mask(bp_list, mask) if name is None: name = f"pwl{model._pwlCounter}" @@ -1705,13 +1798,15 @@ def _add_sos2( ) if links.eq_expr is not None and links.eq_bp is not None: - input_weighted = (lambda_var * links.eq_bp).sum(dim=dim) + eq_bp = _drop_absent(links.eq_bp, links.bp_mask) + input_weighted = (lambda_var * eq_bp).sum(dim=dim) model.add_constraints( links.eq_expr == input_weighted, name=f"{name}{PWL_LINK_SUFFIX}" ) if links.signed_expr is not None and links.signed_bp is not None: - output_weighted = (lambda_var * links.signed_bp).sum(dim=dim) + signed_bp = _drop_absent(links.signed_bp, links.bp_mask) + output_weighted = (lambda_var * signed_bp).sum(dim=dim) _add_signed_link( model, links.signed_expr, @@ -1796,6 +1891,7 @@ def _add_incremental( def _incremental_weighted(bp: DataArray) -> LinearExpression: steps = bp.diff(dim).rename({dim: piece_dim}) steps[piece_dim] = piece_index + steps = _drop_absent(steps, delta_mask) # ``drop=True`` keeps the breakpoint coord from sticking around as a # scalar on ``bp0_term`` — otherwise §11 rejects it as an aux-coord # conflict against the constraint LHS. @@ -1877,13 +1973,15 @@ def _add_disjunctive( ) if links.eq_expr is not None and links.eq_bp is not None: - input_weighted = (lambda_var * links.eq_bp).sum(dim=[SEGMENT_DIM, dim]) + eq_bp = _drop_absent(links.eq_bp, bp_mask) + input_weighted = (lambda_var * eq_bp).sum(dim=[SEGMENT_DIM, dim]) model.add_constraints( links.eq_expr == input_weighted, name=f"{name}{PWL_LINK_SUFFIX}" ) if links.signed_expr is not None and links.signed_bp is not None: - output_weighted = (lambda_var * links.signed_bp).sum(dim=[SEGMENT_DIM, dim]) + signed_bp = _drop_absent(links.signed_bp, bp_mask) + output_weighted = (lambda_var * signed_bp).sum(dim=[SEGMENT_DIM, dim]) _add_signed_link( model, links.signed_expr, @@ -1939,7 +2037,7 @@ def _add_lp( # Use the internal impl so we don't fire a second EvolvingAPIWarning — # ``add_piecewise_formulation`` already warned on entry. - tangents = _tangent_lines_impl(x_expr, x_points, y_points) + tangents = _tangent_lines_impl(x_expr, x_points, y_points, piece_mask) _add_signed_link( model, y_expr, diff --git a/linopy/semantics.py b/linopy/semantics.py index 4f3120570..52f11c3b2 100644 --- a/linopy/semantics.py +++ b/linopy/semantics.py @@ -122,6 +122,52 @@ def _legacy_nan_constant_message(op_kind: str) -> str: ) +_RAGGED_RESOLVE_HINT = ( + "\n Ragged: declare which slots hold a breakpoint —" + "\n `add_piecewise_formulation(..., mask=x_pts.notnull())`" + "\n Data error: `.fillna(value)`" +) + + +def _user_nan_breakpoint_message(context: str) -> str: + """ + User-NaN error text for breakpoint tables. + + The generic §5 message points at `mask=` on a *variable*, which is the + wrong remedy here — breakpoints are plain data, and raggedness is the + reason a NaN usually shows up. Name that case instead. + """ + return ( + f"NaN found in user-supplied {context} (§5). linopy trusts NaN only " + "from its own structural operations (§4): here it cannot tell a " + "shorter curve (a padded slot) from a data error, and guessing would " + "silently change the model." + _RAGGED_RESOLVE_HINT + ) + + +def _legacy_nan_breakpoint_message(context: str) -> str: + """Legacy inferred padding from NaN placement; v1 wants it declared.""" + return ( + f"NaN in user-supplied {context} was silently read as a padded slot " + "(an absent breakpoint) by legacy. Under v1 this raises ValueError — " + "the raggedness has to be declared rather than inferred." + + _RAGGED_RESOLVE_HINT + + _OPT_IN_HINT + ) + + +def check_user_nan_breakpoints(*, context: str = "breakpoints") -> None: + """ + Enforce §5 for a breakpoint table whose padding was not declared. + + v1 raises; legacy warns and lets the caller infer the padding from NaN + placement, preserving pre-v1 behaviour. + """ + if is_v1(): + raise ValueError(_user_nan_breakpoint_message(context)) + warn_legacy(_legacy_nan_breakpoint_message(context)) + + def _legacy_coord_mismatch_message( context: str, dim: str | None = None, diff --git a/test/test_piecewise_constraints.py b/test/test_piecewise_constraints.py index d02e87a7e..5be9bbdbb 100644 --- a/test/test_piecewise_constraints.py +++ b/test/test_piecewise_constraints.py @@ -2032,8 +2032,14 @@ def test_disjunctive_three_pairs(self) -> None: assert f"pwl0{PWL_LAMBDA_SUFFIX}" in m.variables assert f"pwl0{PWL_LINK_SUFFIX}" in m.constraints - def test_disjunctive_interior_nan_raises(self) -> None: - """Disjunctive with interior NaN raises ValueError.""" + def test_disjunctive_interior_nan_raises(self, semantics: str) -> None: + """ + Disjunctive with interior NaN raises ValueError. + + Under v1 the §5 user-NaN check fires first — an interior NaN is + exactly the data error it is there to catch. Legacy still reaches + the layout check further in. + """ m = Model() x = m.add_variables(name="x") y = m.add_variables(name="y") @@ -2046,7 +2052,10 @@ def test_disjunctive_interior_nan_raises(self) -> None: [[0, np.nan, 5], [20, 50, 80]], dims=[SEGMENT_DIM, BREAKPOINT_DIM], ) - with pytest.raises(ValueError, match="non-trailing NaN"): + expected = ( + "NaN found in user-supplied" if semantics == "v1" else "non-trailing NaN" + ) + with pytest.raises(ValueError, match=expected): m.add_piecewise_formulation((x, x_pts), (y, y_pts)) def test_expression_name_fallback(self) -> None: @@ -2104,22 +2113,32 @@ def test_scalar_coord_dropped(self) -> None: @pytest.fixture -def nan_padded_pwl_model() -> Callable[[Method], Model]: - """Factory: NaN-padded per-entity piecewise model parametrized by method.""" +def nan_padded_pwl_model() -> Callable[..., Model]: + """ + Factory: NaN-padded per-entity piecewise model parametrized by method. + + Entity ``b`` has one breakpoint fewer than ``a``, so its last slot is + padding. With ``declare=True`` that absence is passed as ``mask=`` — + the §4 declaration v1 requires — instead of being left for linopy to + infer from the NaN. + """ from linopy.piecewise import breakpoints - def _build(method: Method) -> Model: + def _build(method: Method, declare: bool = False) -> Model: bp_y = pd.DataFrame([[0, 20, 30, 35], [0, 10, 15, np.nan]], index=["a", "b"]) bp_x = pd.DataFrame([[0, 10, 20, 30], [0, 5, 15, np.nan]], index=["a", "b"]) + y_pts = breakpoints(bp_y, dim="entity") + x_pts = breakpoints(bp_x, dim="entity") m = Model() coord = pd.Index(["a", "b"], name="entity") x = m.add_variables(lower=0, upper=20, coords=[coord], name="x") y = m.add_variables(lower=0, upper=40, coords=[coord], name="y") m.add_piecewise_formulation( - (y, breakpoints(bp_y, dim="entity"), "<="), - (x, breakpoints(bp_x, dim="entity")), + (y, y_pts, "<="), + (x, x_pts), method=method, + mask=(x_pts.notnull() & y_pts.notnull()) if declare else None, ) m.add_constraints(x.sel(entity="b") == 10) m.add_objective(-y.sel(entity="b")) @@ -2347,7 +2366,7 @@ def test_convexity_invariant_to_x_direction(self) -> None: @pytest.mark.legacy def test_lp_per_entity_nan_padding( - self, nan_padded_pwl_model: Callable[[Method], Model] + self, nan_padded_pwl_model: Callable[..., Model] ) -> None: """ Per-entity NaN-padded breakpoints with method='lp': padded @@ -2366,7 +2385,7 @@ def test_lp_per_entity_nan_padding( @pytest.mark.parametrize(("solver", "io_api"), _SOS_PATHS) def test_sos2_per_entity_nan_padding( self, - nan_padded_pwl_model: Callable[[Method], Model], + nan_padded_pwl_model: Callable[..., Model], solver: str, io_api: str, ) -> None: @@ -3020,6 +3039,223 @@ def test_eligible_concave_le_returns_ok(self) -> None: assert reason == "" +# =========================================================================== +# Ragged breakpoints — declared absence via mask= (#884) +# =========================================================================== + + +def _ragged_pair() -> tuple[xr.DataArray, xr.DataArray, pd.Index]: + """x/y breakpoints where entity 'b' is one breakpoint short.""" + names = pd.Index(["a", "b"], name="name") + dims = ["name", BREAKPOINT_DIM] + x_pts = xr.DataArray( + [[0.0, 50.0, 100.0], [0.0, 100.0, np.nan]], coords={"name": names}, dims=dims + ) + y_pts = xr.DataArray( + [[0.0, 30.0, 100.0], [0.0, 60.0, np.nan]], coords={"name": names}, dims=dims + ) + return x_pts, y_pts, names + + +def _build_ragged( + method: Method, + *, + declare: bool, + y_values: list[list[float]] | None = None, + active: bool = False, +) -> Model: + x_pts, y_pts, names = _ragged_pair() + if y_values is not None: + y_pts = xr.DataArray( + y_values, coords={"name": names}, dims=["name", BREAKPOINT_DIM] + ) + m = Model() + x = m.add_variables(0, 100, coords=[names], name="x") + y = m.add_variables(coords=[names], name="y") + kwargs: dict[str, Any] = {"method": method} + if declare: + kwargs["mask"] = x_pts.notnull() & y_pts.notnull() + if active: + status = m.add_variables(binary=True, coords=[names], name="s") + kwargs["active"] = status.to_linexpr() + m.add_piecewise_formulation( + (y, breakpoints(y_pts), ">="), (x, breakpoints(x_pts)), name="pw", **kwargs + ) + return m + + +class TestRaggedBreakpointMask: + """ + Ragged curves must be *declared*, not inferred from NaN placement. + + Under v1, §5 rejects a NaN linopy did not create itself; ``mask=`` is + §4's way to say "this slot is absent". Legacy keeps inferring, with a + deprecation warning. Regression for + https://github.com/PyPSA/linopy/issues/884. + """ + + # Every formulation path reaches the padded slot through a different + # expression, and each one used to raise from deep inside arithmetic. + CASES: list[tuple[str, Method, dict[str, Any]]] = [ + ("lp", "lp", {}), + ("sos2", "sos2", {"y_values": [[0.0, 70.0, 20.0], [0.0, 60.0, np.nan]]}), + ( + "incremental", + "incremental", + {"y_values": [[0.0, 70.0, 100.0], [0.0, 60.0, np.nan]]}, + ), + ( + "incremental+active", + "incremental", + {"y_values": [[0.0, 70.0, 100.0], [0.0, 60.0, np.nan]], "active": True}, + ), + ] + + @pytest.mark.v1 + @pytest.mark.parametrize( + ("label", "method", "kwargs"), CASES, ids=[c[0] for c in CASES] + ) + def test_undeclared_nan_raises( + self, label: str, method: Method, kwargs: dict[str, Any] + ) -> None: + with pytest.raises(ValueError, match="NaN found in user-supplied breakpoints"): + _build_ragged(method, declare=False, **kwargs) + + @pytest.mark.v1 + def test_error_names_the_remedy(self) -> None: + """The §5 message must point at mask=, not at the generic fillna advice.""" + with pytest.raises(ValueError) as excinfo: + _build_ragged("lp", declare=False) + message = str(excinfo.value) + assert "mask=" in message + assert "notnull()" in message + + @pytest.mark.parametrize( + ("label", "method", "kwargs"), CASES, ids=[c[0] for c in CASES] + ) + def test_declared_mask_builds( + self, label: str, method: Method, kwargs: dict[str, Any] + ) -> None: + """All formulation paths accept ragged input once it is declared.""" + m = _build_ragged(method, declare=True, **kwargs) + assert len(m.constraints) > 0 + + @pytest.mark.legacy + def test_legacy_warns_but_still_builds(self) -> None: + from linopy.config import LinopySemanticsWarning + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always", LinopySemanticsWarning) + _build_ragged("lp", declare=False) + assert any(issubclass(w.category, LinopySemanticsWarning) for w in caught) + + def test_disjunctive_declared_mask(self) -> None: + names = pd.Index(["a", "b"], name="name") + sdims = ["name", SEGMENT_DIM, BREAKPOINT_DIM] + x_segs = xr.DataArray( + [[[0.0, 30.0], [50.0, 100.0]], [[0.0, 40.0], [np.nan, np.nan]]], + coords={"name": names}, + dims=sdims, + ) + y_segs = xr.DataArray( + [[[0.0, 10.0], [20.0, 60.0]], [[0.0, 15.0], [np.nan, np.nan]]], + coords={"name": names}, + dims=sdims, + ) + m = Model() + x = m.add_variables(0, 100, coords=[names], name="x") + y = m.add_variables(coords=[names], name="y") + m.add_piecewise_formulation( + (y, segments(y_segs)), + (x, segments(x_segs)), + mask=x_segs.notnull() & y_segs.notnull(), + name="pw", + ) + assert len(m.constraints) > 0 + + def test_nan_under_a_present_mask_raises(self) -> None: + """A NaN the mask calls present is a data error, in both semantics.""" + x_pts, y_pts, names = _ragged_pair() + m = Model() + x = m.add_variables(0, 100, coords=[names], name="x") + y = m.add_variables(coords=[names], name="y") + with pytest.raises(ValueError, match="marks as present"): + m.add_piecewise_formulation( + (y, breakpoints(y_pts), ">="), + (x, breakpoints(x_pts)), + mask=xr.ones_like(x_pts, dtype=bool), + name="pw", + ) + + def test_mask_may_hide_a_present_value(self) -> None: + """``mask=`` also drops breakpoints that carry a real value.""" + names = pd.Index(["a", "b"], name="name") + dims = ["name", BREAKPOINT_DIM] + x_pts = xr.DataArray( + [[0.0, 50.0, 100.0], [0.0, 100.0, 200.0]], coords={"name": names}, dims=dims + ) + y_pts = xr.DataArray( + [[0.0, 30.0, 100.0], [0.0, 60.0, 90.0]], coords={"name": names}, dims=dims + ) + mask = xr.DataArray( + [[True, True, True], [True, True, False]], coords={"name": names}, dims=dims + ) + m = Model() + x = m.add_variables(0, 100, coords=[names], name="x") + y = m.add_variables(coords=[names], name="y") + m.add_piecewise_formulation( + (y, breakpoints(y_pts), ">="), + (x, breakpoints(x_pts)), + mask=mask, + name="pw", + ) + assert len(m.constraints) > 0 + + def test_non_broadcastable_mask_raises(self) -> None: + x_pts, y_pts, names = _ragged_pair() + m = Model() + x = m.add_variables(0, 100, coords=[names], name="x") + y = m.add_variables(coords=[names], name="y") + bad = xr.DataArray([True, False], dims=["other"]) + with pytest.raises(ValueError, match="broadcastable"): + m.add_piecewise_formulation( + (y, breakpoints(y_pts), ">="), + (x, breakpoints(x_pts)), + mask=bad, + name="pw", + ) + + @pytest.mark.v1 + def test_tangent_lines_reports_raggedness(self) -> None: + """ + The low-level helper has no mask to declare absence with, so it can + only report — but it must name raggedness rather than let the + generic user-NaN message surface from the chord arithmetic. + """ + x_pts, y_pts, names = _ragged_pair() + m = Model() + x = m.add_variables(0, 100, coords=[names], name="x") + with pytest.raises(ValueError, match="NaN found in user-supplied breakpoints"): + tangent_lines(x, x_pts, y_pts) + + @pytest.mark.v1 + @pytest.mark.parametrize("method", ["lp", "sos2"]) + def test_declared_mask_matches_legacy_oracle( + self, nan_padded_pwl_model: Callable[..., Model], method: Method + ) -> None: + """ + The declared model is the model legacy inferred. + + Same oracle as the legacy-only ``test_*_per_entity_nan_padding`` + tests: f_b(10) on the chord (5,10)→(15,15) is 12.5. + """ + if method == "sos2" and not _SOS_PATHS: + pytest.skip("No SOS-capable solver installed") + m = nan_padded_pwl_model(method, True) + m.solve() + assert abs(float(m.solution.sel({"entity": "b"})["y"]) - 12.5) < 1e-3 + + # =========================================================================== # EvolvingAPIWarning — fires once per session per entry point # =========================================================================== From 2e9642119b6503f2c7e3a0116c336123d552bff1 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Mon, 10 Aug 2026 09:05:19 +0200 Subject: [PATCH 2/2] review: drop convention-paragraph references, compact docstrings Per review on #885: keep docstrings and comments absolute instead of citing convention paragraphs, shorten the _drop_absent docstring to a one-liner, compact the mask parameter docs, and apply the suggested trim of the release-note entry. Co-Authored-By: Claude Fable 5 --- doc/release_notes.rst | 2 +- linopy/piecewise.py | 51 ++++++++---------------------- test/test_piecewise_constraints.py | 13 ++++---- 3 files changed, 21 insertions(+), 45 deletions(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 4d49f0f34..9b8b07639 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -35,7 +35,7 @@ Upcoming Version * Default internal integer labels to ``int32``, cutting memory ~25% and speeding up model build 10-35%. Models exceeding the int32 maximum (~2.1 billion labels) widen to ``int64`` automatically with a ``UserWarning``; pass ``Model(dtypes={"labels": np.int64})`` upfront to avoid the mid-build upcast (exposed read-only via ``Model.dtypes``). * ``add_variables(binary=True, ...)`` now accepts ``lower``/``upper`` bounds, as long as they are 0 or 1. Previously binary bounds could only be set via the ``.lower``/``.upper`` setters after creation. (https://github.com/PyPSA/linopy/issues/776) -* ``add_piecewise_formulation`` gained a ``mask`` parameter declaring which breakpoint slots hold a real breakpoint. It is needed for **ragged** curves — entities with different numbers of breakpoints — which are stored densely with the surplus slots left absent. Under v1 that absence must be declared (``mask=x_pts.notnull()``) rather than read off the NaN padding, since §5 does not let linopy tell a shorter curve from a data error; legacy keeps inferring it, with a ``LinopySemanticsWarning``. Previously ragged curves failed under v1 with a generic "NaN found in a user-supplied constant" raised from deep inside the formulation's arithmetic, on all of the ``lp``, ``sos2``, ``incremental`` and disjunctive paths. (https://github.com/PyPSA/linopy/issues/884) +* ``add_piecewise_formulation`` gained a ``mask`` parameter declaring which breakpoint slots hold a real breakpoint. It is needed for **ragged** curves — entities with different numbers of breakpoints — which are stored densely with the surplus slots left absent. Under v1 that absence must be declared (``mask=x_pts.notnull()``) rather than read off the NaN padding. (https://github.com/PyPSA/linopy/issues/884) * ``add_piecewise_formulation`` gained an ``active_fill`` parameter that gates a partial ``active`` (defined over a subset of the indexed dimension, or masked) as always-active (``1``) or always-off (``0``); without it, a partial ``active`` — which was previously zeroed silently — now raises. Useful when one formulation mixes gated and ungated entities (e.g. committable and non-committable units sharing a ``status``). ``active_fill`` is transitional and will be removed once v1 semantics make ``active.reindex(coords).fillna(value)`` sufficient. (https://github.com/PyPSA/linopy/issues/796) *Documentation* diff --git a/linopy/piecewise.py b/linopy/piecewise.py index 339ff9359..2061c35e9 100644 --- a/linopy/piecewise.py +++ b/linopy/piecewise.py @@ -836,9 +836,6 @@ def tangent_lines( "entirely with " '`warnings.filterwarnings("ignore", category=linopy.EvolvingAPIWarning)`.', ) - # No mask to declare absence on this low-level helper, so a ragged curve - # can only be reported (§5), not resolved — say so here rather than let - # the generic user-NaN message surface from the chord arithmetic. if bool(_coerce_breaks(x_points).isnull().any()) or bool( _coerce_breaks(y_points).isnull().any() ): @@ -997,16 +994,7 @@ def _paired_valid_points(*points: DataArray) -> DataArray: def _drop_absent(values: DataArray, mask: DataArray | None) -> DataArray: - """ - Make a breakpoint table safe to use as a *constant* operand. - - Absent slots are marked with ``NaN`` (§2), but as soon as the table - multiplies a variable it is an ordinary constant, and §5 rejects NaN - there — provenance is gone by that point. The absence is already - carried by the variable, which was created with ``mask=`` (§4), so it - propagates on its own (§6) and the coefficient at that slot is never - read. Replace it with a neutral zero. - """ + """Zero out masked-absent slots; the variable's own mask already excludes them.""" if mask is None: return values return values.where(mask, 0.0) @@ -1019,14 +1007,12 @@ def _resolve_breakpoint_mask( Settle which slots hold a real breakpoint, and align the values to it. A ragged curve is stored densely along ``BREAKPOINT_DIM`` with the - surplus slots absent, which §2 encodes as ``NaN``. §5 forbids reading - that marker off *user* data — a shorter curve and a data error are - indistinguishable — so the caller declares the absence through §4's - ``mask=`` and this returns it. Without a declaration, v1 raises and - legacy keeps inferring from NaN placement. - - The returned arrays carry ``NaN`` exactly where the mask says absent, - so ``isnull()`` stays the single predicate (§3) downstream. + surplus slots absent. A shorter curve and a data error are + indistinguishable from NaN alone, so the caller declares the absence + with ``mask=``; without a declaration v1 raises and legacy keeps + inferring from NaN placement. The returned arrays carry ``NaN`` + exactly where the mask says absent, so ``isnull()`` stays the single + absence predicate downstream. """ combined_null = bp_list[0].isnull() for bp in bp_list[1:]: @@ -1263,22 +1249,13 @@ def add_piecewise_formulation( mask : MaskLike, optional Which breakpoint slots hold a real breakpoint — ``True`` where one exists, ``False`` where it is absent. Shaped like the breakpoint - arrays (entity dims × ``_breakpoint``), or anything that broadcasts - against them. - - Needed only for **ragged** curves, where entities have different - numbers of breakpoints. These are stored densely with the surplus - slots left absent, and under v1 that absence has to be declared - rather than read off the data: a shorter curve and a stray NaN look - identical, so linopy refuses to guess (see the convention, §4/§5). - For breakpoints that are already NaN-padded, the declaration is - ``mask=x_pts.notnull()``. - - Slots marked absent are excluded from the formulation: no auxiliary - variable is created for them and no constraint references them. - Passing a mask that hides a *present* value is allowed and drops - that breakpoint; a NaN at a slot the mask calls present is a data - error and raises. + arrays, or anything that broadcasts against them. Needed for + **ragged** curves (entities with different numbers of breakpoints), + stored densely with the surplus slots left absent: under v1 that + absence must be declared rather than read off the NaN padding, e.g. + ``mask=x_pts.notnull()``. Absent slots get no auxiliary variable + and no constraint. A mask hiding a present value drops that + breakpoint; a NaN at a slot the mask calls present raises. name : str, optional Base name for generated variables/constraints. diff --git a/test/test_piecewise_constraints.py b/test/test_piecewise_constraints.py index 5be9bbdbb..365ff962b 100644 --- a/test/test_piecewise_constraints.py +++ b/test/test_piecewise_constraints.py @@ -2036,7 +2036,7 @@ def test_disjunctive_interior_nan_raises(self, semantics: str) -> None: """ Disjunctive with interior NaN raises ValueError. - Under v1 the §5 user-NaN check fires first — an interior NaN is + Under v1 the user-NaN check fires first — an interior NaN is exactly the data error it is there to catch. Legacy still reaches the layout check further in. """ @@ -2118,9 +2118,8 @@ def nan_padded_pwl_model() -> Callable[..., Model]: Factory: NaN-padded per-entity piecewise model parametrized by method. Entity ``b`` has one breakpoint fewer than ``a``, so its last slot is - padding. With ``declare=True`` that absence is passed as ``mask=`` — - the §4 declaration v1 requires — instead of being left for linopy to - infer from the NaN. + padding. With ``declare=True`` that absence is passed as ``mask=``, as + v1 requires, instead of being left for linopy to infer from the NaN. """ from linopy.piecewise import breakpoints @@ -3088,8 +3087,8 @@ class TestRaggedBreakpointMask: """ Ragged curves must be *declared*, not inferred from NaN placement. - Under v1, §5 rejects a NaN linopy did not create itself; ``mask=`` is - §4's way to say "this slot is absent". Legacy keeps inferring, with a + Under v1 a NaN linopy did not create itself is rejected; ``mask=`` is + the way to say "this slot is absent". Legacy keeps inferring, with a deprecation warning. Regression for https://github.com/PyPSA/linopy/issues/884. """ @@ -3123,7 +3122,7 @@ def test_undeclared_nan_raises( @pytest.mark.v1 def test_error_names_the_remedy(self) -> None: - """The §5 message must point at mask=, not at the generic fillna advice.""" + """The error message must point at mask=, not at the generic fillna advice.""" with pytest.raises(ValueError) as excinfo: _build_ragged("lp", declare=False) message = str(excinfo.value)