Skip to content

Commit a4b822d

Browse files
jkiviluoclaude
andcommitted
fix(outputs): broadcast scalar existing mixed with period-Map siblings
A unit with a constant `existing` capacity sharing an entity class with a period-Map sibling surfaces from SpineDbReader as a single frame `[name, x, value]` where the constant unit's row carries a NULL index (`x = null`; the Map index is named `x` by Spine's silent default). `_per_entity_param_lf` classified `is_scalar` per-FRAME (does an index column exist?) instead of per-ROW, so the null-index scalar became an explicit `(e, d=null)` row that never joined the period grid in `_resolve_per_period_lf` and was silently filled with 0. This zeroed the output-facing `p_entity_all_existing` for such units, surfacing as negative VRE curtailment (`potential - flow` with `potential` derived from a 0 capacity), plus 0 in the capacity / capacity-factor / pre- existing fixed-cost reports. Output-only: the LP flow bound uses `p_process_existing_count` (scalar- correct), and `p_entity_all_existing` enters the objective only as an opt-in constant term (off by default) — dispatch, investments, flows, and the optimal objective are unchanged. Fix: per-row scalar detection `is_scalar = period_col.is_null()` in both mirror implementations (`_derived_npv`, `_derived_existing`). Adds a regression test exercising the mixed scalar+Map shape via InMemoryReader. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent bbbe082 commit a4b822d

3 files changed

Lines changed: 129 additions & 2 deletions

File tree

flextool/engine_polars/_derived_existing.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,11 +242,19 @@ def _per_entity_param_lf(source: "InputSource",
242242
extra = [c for c in cols if c not in ("name", "value")]
243243
period_col = "period" if "period" in extra else (extra[0] if extra else None)
244244
if period_col is not None:
245+
# Per-ROW scalar detection — see the matching helper in
246+
# ``_derived_npv._per_entity_param_lf`` for the full rationale.
247+
# A constant ``existing`` (etc.) stored in an entity class
248+
# that ALSO carries period-Map values for sibling entities
249+
# surfaces with a NULL index (``x = null``). That row must
250+
# broadcast across the period universe (``is_scalar=True``),
251+
# not be emitted as an explicit ``(e, d=null)`` row that
252+
# never joins the period grid and silently zeroes out.
245253
parts.append(df.lazy().select(
246254
alias_to_axis("name", "e"),
247255
alias_to_axis(period_col, "d"),
248256
pl.col("value").cast(pl.Float64, strict=False),
249-
pl.lit(False).alias("is_scalar"),
257+
pl.col(period_col).is_null().alias("is_scalar"),
250258
))
251259
else:
252260
parts.append(df.lazy().select(

flextool/engine_polars/_derived_npv.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,23 @@ def _per_entity_param_lf(source: "InputSource",
201201
extra = [c for c in cols if c not in ("name", "value")]
202202
period_col = "period" if "period" in extra else (extra[0] if extra else None)
203203
if period_col is not None:
204+
# Per-ROW scalar detection. A scalar value stored alongside
205+
# Map values in the same entity class surfaces with a NULL
206+
# index ((e.g. ``x = null``) — Spine emits one ``x`` column
207+
# for the whole class, so a unit with a constant ``existing``
208+
# while a sibling unit carries a period-Map shows up as a
209+
# null-period row. Such rows must broadcast across the
210+
# period universe (``is_scalar=True``), NOT be treated as an
211+
# explicit ``(e, d=null)`` row that fails to join the period
212+
# grid and silently fills 0. Classifying by ``period_col``'s
213+
# null-ness per row (rather than per frame) covers the mixed
214+
# scalar+Map class.
204215
parts.append(df.lazy().select(
205216
alias_to_axis("name", "e"),
206217
alias_to_axis(pl.col(period_col).cast(pl.Utf8, strict=False),
207218
"d"),
208219
pl.col("value").cast(pl.Float64, strict=False),
209-
pl.lit(False).alias("is_scalar"),
220+
pl.col(period_col).is_null().alias("is_scalar"),
210221
))
211222
else:
212223
parts.append(df.lazy().select(
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
"""Regression — per-entity scalar value mixed with period-Map values in
2+
the same entity class.
3+
4+
When one unit carries a *constant* ``existing`` capacity while a sibling
5+
unit in the same class carries a *period-Map* ``existing``, the Spine
6+
reader returns a single frame with one shared index column (named ``x``
7+
by Spine's silent default) where the constant unit's row has a NULL
8+
index::
9+
10+
name x value
11+
wind_scalar null 31.5 <- scalar, broadcast across periods
12+
wind_map p2024 10.8 <- period-Map row
13+
wind_map p2025 22.8
14+
15+
``_per_entity_param_lf`` must classify the null-index row as a SCALAR
16+
(``is_scalar=True``) so ``_resolve_per_period_lf`` broadcasts it across
17+
the period universe. The historical bug classified the whole frame by
18+
the *presence* of the index column, so the null-index scalar became an
19+
explicit ``(e, d=null)`` row that never joined the period grid and was
20+
silently filled with 0 — surfacing downstream as a zeroed-out existing
21+
capacity (e.g. negative VRE curtailment = ``potential - flow`` with
22+
``potential`` computed from a 0 capacity).
23+
24+
Both mirror implementations of the helper are covered.
25+
"""
26+
from __future__ import annotations
27+
28+
import polars as pl
29+
import pytest
30+
31+
from flextool.engine_polars import _derived_existing as de
32+
from flextool.engine_polars import _derived_npv as dn
33+
34+
35+
def _mixed_existing_source():
36+
"""InMemoryReader whose ``unit.existing`` mixes a scalar (null index)
37+
with a period-Map, mirroring ``SpineDbReader.parameter`` output."""
38+
from flextool.engine_polars._inmemory_reader import InMemoryReader
39+
40+
frame = pl.DataFrame(
41+
{
42+
"name": ["wind_scalar", "wind_map", "wind_map", "wind_map"],
43+
"x": [None, "p2024", "p2025", "p2026"],
44+
"value": [31.5, 10.8, 22.8, 22.8],
45+
}
46+
)
47+
entities = {"unit": pl.DataFrame({"name": ["wind_scalar", "wind_map"]})}
48+
return InMemoryReader(entities, {("unit", "existing"): frame})
49+
50+
51+
@pytest.mark.parametrize("module", [de, dn], ids=["derived_existing", "derived_npv"])
52+
def test_null_index_row_is_scalar(module):
53+
"""A null-index row must be flagged ``is_scalar=True``; the Map rows
54+
must stay ``is_scalar=False`` with their explicit period."""
55+
src = _mixed_existing_source()
56+
out = module._per_entity_param_lf(src, "existing").collect()
57+
58+
scalar_row = out.filter(pl.col("e") == "wind_scalar")
59+
assert scalar_row.height == 1
60+
assert scalar_row["is_scalar"][0] is True
61+
assert scalar_row["d"][0] is None
62+
assert scalar_row["value"][0] == pytest.approx(31.5)
63+
64+
map_rows = out.filter(pl.col("e") == "wind_map")
65+
assert map_rows.height == 3
66+
assert not any(map_rows["is_scalar"].to_list())
67+
assert set(map_rows["d"].to_list()) == {"p2024", "p2025", "p2026"}
68+
69+
70+
@pytest.mark.parametrize("module", [de, dn], ids=["derived_existing", "derived_npv"])
71+
def test_scalar_broadcasts_across_period_grid(module):
72+
"""The scalar must resolve to its constant value at EVERY period in
73+
the grid, not be zero-filled. This is the load-bearing assertion:
74+
the bug zeroed ``wind_scalar`` at every period."""
75+
src = _mixed_existing_source()
76+
per = module._per_entity_param_lf(src, "existing")
77+
78+
grid = pl.LazyFrame(
79+
{
80+
"e": ["wind_scalar", "wind_scalar", "wind_map", "wind_map"],
81+
"d": ["p2025", "p2026", "p2025", "p2026"],
82+
}
83+
)
84+
resolved = module._resolve_per_period_lf(per, grid, fill=0.0).collect()
85+
86+
scalar_res = resolved.filter(pl.col("e") == "wind_scalar").sort("d")
87+
assert scalar_res["value"].to_list() == pytest.approx([31.5, 31.5])
88+
89+
map_res = resolved.filter(pl.col("e") == "wind_map").sort("d")
90+
assert map_res["value"].to_list() == pytest.approx([22.8, 22.8])
91+
92+
93+
def test_pre_existing_broadcasts_scalar_to_all_periods():
94+
"""End-to-end through ``p_entity_pre_existing_lf``: the scalar unit's
95+
pre-existing capacity must be present (non-zero) at the active
96+
period, matching the period-Map unit's treatment."""
97+
src = _mixed_existing_source()
98+
pre = de.p_entity_pre_existing_lf(
99+
src, active_solve="y2025", period_in_use=["p2025"]
100+
).collect()
101+
102+
scalar_res = pre.filter(pl.col("e") == "wind_scalar")
103+
assert scalar_res.height == 1
104+
assert scalar_res["value"][0] == pytest.approx(31.5)
105+
106+
map_res = pre.filter(pl.col("e") == "wind_map")
107+
assert map_res.height == 1
108+
assert map_res["value"][0] == pytest.approx(22.8)

0 commit comments

Comments
 (0)