bug: DP backward-induction cost-basis threading is dead code — anti-cycling floor evaluates every discharge against frozen initial cost basis

[johanzander]

Summary

While investigating #231 (whether the whole-day profitability gate is still needed), found that the per-action anti-cycling floor in _compute_reward — the mechanism intended to block individually-unprofitable discharges during backward induction — is fed a frozen initial_cost_basis for every period after the first, not the true, path-dependent FIFO cost basis. The side-channel meant to thread cost basis forward through the DP is dead code due to a loop-order bug, and its return value is discarded by the caller anyway.

Evidence

core/bess/dp_battery_algorithm.py, _run_dynamic_programming (lines 659-889):

C = np.full((horizon + 1, len(soe_levels)), initial_cost_basis)   # line 699

for t in reversed(range(horizon)):        # line 705 — descends horizon-1 -> 0
    for i, soe in enumerate(soe_levels):
        ...
        reward, new_cost_basis = _compute_reward(
            ...,
            cost_basis=C[t, i],           # line 769 — read for period t
        )
        ...
        if best_action != 0 and t + 1 < horizon:
            ...
            C[t + 1, next_i] = best_new_cost_basis   # line 889 — write to period t+1

Because the outer loop counts down from horizon-1 to 0, period t+1 is fully processed in an earlier loop iteration than period t. The write at line 889 (C[t+1, ...]) targets a slot that has already been read and consumed by the time it's written — it can never be read back by anything in this function. C[t, i] is therefore just the constant initial_cost_basis for every (t, i) evaluated during the entire backward pass, never a path-dependent value.

Confirmed the return value is also discarded by the sole caller, optimize_battery_schedule (line 1100):

V, policy, _, _ = _run_dynamic_programming(...)

A second, correctly forward-threaded cost basis is computed afterward in a separate replay loop (lines 1129-1177, ascending for t in range(horizon)), which is what feeds the reported cost_basis in period data. But by then the action at each period was already locked in by the (cost-basis-blind) backward pass. If replaying with the true cost basis would make the already-chosen discharge fail the anti-cycling floor, the code explicitly does not reject or reconsider it — it just keeps the current basis and executes the discharge anyway:

# lines 1159-1162
# _compute_reward returns (-inf, cost_basis) for a blocked discharge; when
# replaying the already-chosen policy action just keep the current basis.
if reward == float("-inf"):
    new_cost_basis = current_cost_basis

Practical consequence

battery_wear_cost = 0.0 for discharge rewards (line 359), so a discharge's per-period dollar reward has no wear cost baked in during the backward pass — the anti-cycling floor (comparing against cost basis) is the only thing preventing an opportunistic discharge. That floor is correct for a run's very first decision (where cost_basis genuinely equals initial_cost_basis), but for any later period in a multi-cycle schedule — charge now, discharge later at a different price — it's evaluated against a stale basis, not the true one the executed path would accumulate. The existing regression suite already builds and expects multi-window schedules with an initial_cost_basis distinct from later charge/discharge windows as the normal case (core/bess/tests/unit/test_optimization_algorithm.py:335-406), not an edge case — so this isn't a narrow corner condition.

Why this matters for #231

#231's fix (narrowly scoped) corrects the whole-day profitability gate's baseline (solar_only_cost). That gate remains a legitimate backstop specifically because this per-action floor can't be trusted beyond period 0 of a multi-cycle run. This issue is the deeper reason a whole-horizon check is still needed at all, and is being filed separately given its larger scope: any fix here likely needs cost_basis folded into the DP's state space (or an equivalent correct threading), and will very likely require regenerating several pinned test fixtures that currently encode the buggy economics (per the precedent in #209's PR, which had the same kind of fallout when a related reward-function bug was fixed).

Suggested investigation direction (not verified as sufficient — needs its own diagnosis)

• Quantify how often this actually changes which action is chosen in practice (vs. being inert because most single re-optimization runs don't involve enough distinct charge/discharge price windows for the frozen basis to matter).
• Consider whether cost basis needs to become part of the DP's state (bigger, more expensive state space) or whether a cheaper approximation (e.g., re-running backward induction with cost basis from a first forward pass) is sufficient.

Related

• Surfaced during brainstorming for bug: optimizer profitability gate uses wrong baseline, falls back to all-IDLE schedule (blocks discharge) on high-solar days #231 (bess-analyst investigation + independent verification against source, 2026-07-04).
• Related reward-function bug history: fix: anti-cycling discharge gate over-values stored energy when solar already covers all load #204/fix: anti-cycling discharge gate over-values stored energy in solar surplus #209 (anti-cycling gate over-crediting), Fix solar-export over-crediting + faithful control vocabulary (binary store/export, continuous reconstruction) (#145) #146/Revert #146: binary store/export broke no-solar price arbitrage #151/Re-land corrected #146: solar-export over-crediting fix + DP IDLE-action fix #152 (solar-export over-crediting + missing-IDLE-action regression).

[bess-manager-claude-bot]

Summary: The backward-induction DP loop's cost-basis threading doesn't execute correctly due to backward enumeration order — the anti-cycling floor uses a frozen initial basis instead of the true path-dependent FIFO basis for all periods after period 0. Evidence and code paths provided above confirm this is dead code whose written value is never read back, and whose sole caller discards the return value entirely.

Confirmed: A second, forward-threaded cost basis is computed afterward and used for reporting, but the action at each period was already locked in by the cost-basis-blind backward pass, so the true basis can't influence the chosen policy.

Reply @claude-bot analyze to start deep code-level analysis.

[johanzander]

Root cause — confirmed against current main (post-#231-merge, HEAD d74b99f)

Traced every read/write of C in _run_dynamic_programming (core/bess/dp_battery_algorithm.py) — confirmed the bug exactly as described, no other read/write path found:

• :699 C = np.full((horizon+1, len(soe_levels)), initial_cost_basis)
• :705 for t in reversed(range(horizon)): — descends horizon-1 → 0
• :769 cost_basis=C[t, i] fed to _compute_reward (the profitability floor)
• :884-889 if best_action != 0 and t+1 < horizon: C[t+1, next_i] = best_new_cost_basis
• :1101 V, policy, _, _ = _run_dynamic_programming(...) — the DP's own cost-basis array is discarded by its sole caller

Since the outer loop descends, period t+1 is fully processed before the iteration that writes C[t+1, ...] runs — that write is never read back. C[t, i] is provably the constant initial_cost_basis for every (t, i) throughout the entire backward pass, not just for period 0.

A separate, correctly forward-threaded cost basis is computed afterward in an ascending replay loop (:1130-1193), used for reporting — but by then the backward pass has already locked in the policy using the wrong floor. If replay finds the true cost basis would make an already-chosen discharge unprofitable, it does not reconsider — it executes the discharge anyway and just freezes the basis (:1160-1163).

Empirical impact

Built a corrected 2-pass harness (pass 1 = today's behavior, pass 2 = backward induction re-run using pass 1's true forward-threaded per-period entering cost basis) and diffed chosen actions against current behavior across 19 pinned fixtures (all unit-test scenarios + 13 JSON fixtures ≤ 60 periods): zero action differences in every case. True-vs-frozen cost basis deviations of 1–2 SEK/kWh are common but never flip a decision — Nordic buy/sell spreads in these fixtures dominate the error.

Constructed an adversarial case to confirm the mechanism is real, not just theoretical: a stale expensive initial_cost_basis + a single narrow late-horizon arbitrage window. With the frozen floor, the DP never charges at all (the future discharge never gets credited during backward induction, so upstream charging looks pointless). With the corrected cost basis, the DP monetizes the cycle correctly (V[0] jumps from -1.0 to +6.7 to +7.6 SEK depending on the setup).

Conclusion: real defect, not inert by design, but no evidence it has affected any real schedule to date — it's a latent bug most likely to surface on a re-optimization run where the battery holds expensive stored energy from a prior cycle and a new, cheap, narrow arbitrage window opens later in the same horizon.

Why a cheap local patch doesn't work

Considered fixing only the forward replay loop (cheap, O(horizon), no second backward pass) instead of a full two-pass fix. This doesn't work: the bug's damage happens during the backward pass, in the value function itself. When a discharge is wrongly blocked by the frozen floor, it drops out of the max() that computes V[t,i], so periods before it never see the true future payoff of an earlier cheap charge — that's exactly why the adversarial case shows the DP declining to charge at all. Replay only walks a policy the backward pass already fixed; it cannot retroactively make the DP charge. A second, corrected backward pass (or an equivalent full re-evaluation) is structurally required to fix this — there's no cheaper patch that preserves correctness.

Recommended fix (designed, not yet implemented)

Two-pass backward induction, no new DP state dimension:

1. _run_dynamic_programming (:659-907): delete the dead C grid (:699, :709, :884-889, and its slot in the return tuple at :907). Add cost_basis_by_period: list[float] | None = None; at the top of each outer-loop iteration use cb_t = cost_basis_by_period[t] if cost_basis_by_period is not None else initial_cost_basis everywhere C[t, i] was read. Return (V, policy, stored_period_data).
2. optimize_battery_schedule (:1030-1229): run the existing structure twice — pass 1 identical to today, forward-replay to capture the true entering cost basis per period, pass 2 re-run with that as input, then forward-replay again for the final output (shadow price, economic summary, hourly_results).

Verified: _run_dynamic_programming has exactly one caller, so the signature change is safe. No pinned test fixture regeneration expected — none assert exact DP-level cost-basis values or depend on the buggy path.

Rejected alternative: discretizing cost basis into the DP state space (fully "correct," no approximation) — multiplies state count by however many cost-basis buckets are needed (20-50+), so it costs 20-50x rather than 2x. Strictly worse given the bug is empirically a narrow edge case.

Why this isn't being implemented yet

The two-pass fix roughly doubles _run_dynamic_programming's runtime, which is already a pure-Python nested loop (~240 SOE states × ~150 power levels, no numpy vectorization) — benchmarked at ~11.5s per call at production's actual 96-period/1-day horizon, ~22.8s at a 192-period/2-day horizon. Doubling that (~23s / ~45s per hourly re-optimization) isn't something we want to ship as-is. Filed #236 to vectorize that hot loop first, so the two-pass cost becomes negligible. This fix is designed and ready to implement as soon as #236 lands — tracking here, not closing.

Related

• perf: vectorize DP backward-induction hot loop in dp_battery_algorithm.py #236 (blocks this fix — vectorize the DP hot loop)
• bug: optimizer profitability gate uses wrong baseline, falls back to all-IDLE schedule (blocks discharge) on high-solar days #231 (whole-day profitability gate, already merged — confirmed orthogonal, doesn't touch C/cost-basis code)
• fix: anti-cycling discharge gate over-values stored energy when solar already covers all load #204/fix: anti-cycling discharge gate over-values stored energy in solar surplus #209, Fix solar-export over-crediting + faithful control vocabulary (binary store/export, continuous reconstruction) (#145) #146/Revert #146: binary store/export broke no-solar price arbitrage #151/Re-land corrected #146: solar-export over-crediting fix + DP IDLE-action fix #152 (prior reward-function bug history)

[johanzander]

Scope note for when this is implemented: is cost_basis the right gate at all?

While diagnosing a separate report (#126 -- evening discharge shrinking between two same-day re-optimizations, root-caused in #240 to a related-but-distinct gap in the reward model's flow accounting), we ended up questioning something more fundamental about the anti-cycling mechanism this issue is about fixing the threading for. Worth assessing before just correctly threading cost_basis forward, because fixing the threading bug doesn't necessarily fix the underlying design.

Two different quantities are doing similar jobs, only one of them is theoretically sound

The DP computes two distinct things that both claim to represent "the value of stored energy":

1. shadow_price (dp_battery_algorithm.py:1180-1189):

# Shadow price = marginal opportunity value of stored energy (dV/dSoE),
# by backward difference at the chosen grid level i ...
if i > 0:
    period_data.decision.shadow_price = float(
        (V[t, i] - V[t, i - 1]) / SOE_STEP_KWH
    )

This is the marginal value function derivative from the DP's own value table V. In a correctly-computed Bellman backward induction, this is by construction forward-looking and already accounts for every future opportunity to use that kWh -- because every reward the recursion evaluates already includes V[t+1]. This is the theoretically correct object for gating a hold-vs-use decision, and it's already being used correctly elsewhere (e.g. battery_system_manager.py's solar_export_discharge_rate: "Returns 100 (allow discharge) when buy_price * eff_d >= shadow_price").

2. cost_basis (dp_battery_algorithm.py:316-411, the subject of this issue):

effective_cost_basis = cost_basis
if current_sell_price > current_buy_price:
    effective_cost_basis = max(effective_cost_basis, current_sell_price)
else:
    ...
    if excess_solar > POWER_TOLERANCE_KW and capacity_after_discharge > SOE_STEP_KWH:
        effective_cost_basis = max(effective_cost_basis, current_sell_price)

if effective_value_per_kwh_stored <= effective_cost_basis:
    return float("-inf"), cost_basis

cost_basis is a blended average historical cost of everything currently in the battery (weighted-average-cost inventory accounting, not a marginal/forward quantity). It's used as a second, independent veto on top of the value function: "block this discharge unless its value beats the average cost of what's stored," evaluated inside _compute_reward -- i.e. inside the same backward induction that's already computing V and could derive shadow_price at that point instead.

Why this is architecturally suspect, not just cosmetically odd

In a correctly-computed Bellman recursion, the value function already encodes whether it's better to hold a kWh for a future opportunity or use/export it now -- that's the entire point of backward induction. A separate "cost basis floor" bolted onto the per-transition reward is either:

• redundant (if V is correct, it would never choose an action shadow_price alone wouldn't also justify), or
• actively wrong (if cost_basis disagrees with what V would say, the floor can block a genuinely value-maximizing action, or -- as this issue documents -- silently do nothing because it's frozen).

Given this issue's finding that cost_basis fed into _compute_reward during the actual backward pass is frozen at initial_cost_basis (just cycle_cost_per_kwh, e.g. 0.04 EUR/kWh) for every period all day -- not the true path-dependent average -- the schedules this system has been producing were decided against a near-meaningless floor, while the correctly-threaded average (computed only in the separate forward "replay" pass used for reporting, e.g. period-data economics shown in the dashboard) never actually influenced which actions got chosen. So today's behavior is a coincidence: the frozen 0.04 EUR/kWh floor is usually low enough not to block anything, which happens to look reasonable, but that's not evidence the mechanism is sound -- it's evidence it's currently mostly inert.

On "isn't stored solar just free, always favorable to export"

Worth stating precisely, since it motivates why some form of opportunity-cost accounting is legitimate here, even though the current implementation of it looks wrong: solar is not free the instant it's generated -- it has a real opportunity cost equal to whatever it could be sold for right now. Storing it for later use (self-consumption or export) is only correct if the later use beats that forgone sale, net of round-trip efficiency loss (~92% here: 0.97 charge x 0.95 discharge). So solar_opportunity_cost = battery_charged * current_sell_price (line 408-411) is a defensible idea. The flaw isn't the idea -- it's implementing it as a blended average bolted onto the reward function as a parallel veto, instead of trusting the value function (which, if V is computed correctly, already prices this trade-off in via shadow_price) to be the sole arbiter.

Suggested scope for this issue's fix

When picking this up, in addition to fixing the loop-order threading bug as originally scoped, evaluate whether effective_cost_basis/cost_basis should be:

• (a) removed from _compute_reward entirely, relying purely on the (correctly-threaded, once fixed) value function / shadow_price to gate discharge profitability, or
• (b) kept as a guard-rail heuristic, but rebuilt on shadow_price rather than a blended historical average, or
• (c) kept as-is (blended average, correctly threaded) if there's a concrete scenario where the value function alone is known to be insufficient (e.g. numerical/discretization edge cases like bug: DP reward model treats surplus discharge as forced export, ignoring load_first self-throttling — causes avoidable grid import at SOE step boundaries #240) -- in which case that scenario should be documented and tested explicitly, since right now the rationale for having a second gate at all isn't written down anywhere.

Whichever direction, this needs its own regression coverage distinguishing "value function alone would have made the same call" from "the cost-basis floor changed the outcome," since right now we have no way to tell how often it actually matters.

Related

• bug: DP reward model treats surplus discharge as forced export, ignoring load_first self-throttling — causes avoidable grid import at SOE step boundaries #240 -- a distinct but adjacent reward-model gap in the same function (_compute_reward's flow-balance treats any discharge beyond home_consumption as forced grid export, even for load-following/load_first intents that physically self-throttle).
• Belpex H integration? #126 -- the user report that prompted this investigation.

[johanzander]

Fixed as part of the DP guardrail removal (526656d, shipped to beta directly, forward-ported to main via #253): the dead C cost-basis-threading grid this issue identified is removed entirely — _run_dynamic_programming now passes the constant initial_cost_basis directly instead of maintaining an array that was never correctly read back. Confirmed on current main: no more C = np.full(...) grid or per-period C[t, i] writes. Live in main and the v9.9.0b10 beta release.