Fixup start costs

[maurerle]

User description

Related Issue

relates to #695

Description

This fixes calculation of start costs by using get_starting_costs in execute_current_dispatch.

This also restructures calculate_generation_cost to be part of execute_current_dispatch as this in the "general" part which should be part of all the specific implementations.

However, this still needs some fixes in the various strategies, I guess.

Checklist

• Documentation updated (docstrings, READMEs, user guides, inline comments, doc folder updates etc.)
• New unit/integration tests added (if applicable)
• Changes noted in release notes (if any)
• Consent to release this PR's code under the GNU Affero General Public License v3.0

Additional Notes (optional)

---

PR Type

Bug fix, Enhancement, Tests, Documentation

---

Description

• Book start-up costs from final dispatch

• Make cost booking idempotent across markets

• Update rewards to reuse booked costs

• Add tests and dashboard/outputs updates

---

Diagram Walkthrough

flowchart LR
  A["Unit dispatch (execute_current_dispatch)"]
  B["Finalize outputs[energy]"]
  C["PowerPlant._book_start_costs()"]
  D["calculate_generation_cost()"]
  E["outputs[energy_start_costs] & outputs[energy_generation_costs]"]
  F["Strategies calculate_reward()"]
  G["Unit operator exports unit_dispatch"]
  H["Grafana + docs include start costs"]
  A -- "runs per market tick" --> B
  B -- "detect off->on transitions" --> C
  B -- "compute variable costs" --> D
  C -- "write" --> E
  D -- "write" --> E
  E -- "read costs" --> F
  E -- "persist" --> G
  G -- "visualize/document" --> H

Loading

File Walkthrough

	Relevant files
Enhancement	3 files base.py Trigger cost calc and improve operation lookback                  +26/-37  units_operator.py Include start costs in dispatched outputs                                +1/-1      storage.py Remove storage startup costs and recompute generation costs +5/-50
Bug fix	3 files flexable.py Read booked costs to avoid double counting                              +15/-23  learning_strategies.py Replace recomputed startup costs with booked series            +18/-25  powerplant.py Add idempotent start-cost booking and lookback                      +80/-1
Tests	3 files test_baseunit.py Clarify idempotent generation-cost behavior in tests          +9/-4      test_powerplant.py Add comprehensive startup-cost booking test suite                +189/-0  test_storage.py Remove downtime warm/hot validation test cases                      +0/-12
Documentation	2 files ASSUME.json Show start-up costs and profit correctly                                  +4/-4      outputs.rst Document `energy_start_costs` in unit outputs                        +1/-1

---

[kim-mskw]

Did someone do a performance check here? I remember that we explictly did not recalcuate the marginal costs becasue it was slowing down the simualtio quite a bit.

[maurerle]

I checked it shortly with example_03 and did not see a huge performance decrease - but would like to investigate this further

[kim-mskw]

So Charging cannot have ramping constraints or costs?

[maurerle]

This is not about ramping - it is about start cost - so a price one would pay per start of the hydro-storage-turbines.

I actually think that start_cost do not really make sense for storages, especially batteries and would leave this option for powerplants only?
An alternative is to model the storage with a cycling cost with opexPerChargeOperation and opexPerDischargeOperation as done in FINE:
https://github.com/FZJ-IEK3-VSA/FINE/blob/1b6c40cc56bc790cf15cf69de19847725d42185a/fine/storage.py#L51

• AMIRIS does not have something like this from what I have seen.

[kim-mskw]

If function is truly idempotent than why not call it after the bid fomrualtion as well, and after the dispatch they are overwritten?

[maurerle]

idempotent means that it does not break anything, but it still costs performance to caculate it here as well.
But yes, maybe calling it here would solve it.

[kim-mskw]

Why did you not carry over the "/2"?

[maurerle]

This is now working fine:

costs are available as a dedicated time series.

what is left:

• test if this creates issues for learning
• further testing

[codecov]

Codecov Report

❌ Patch coverage is 86.95652% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.17%. Comparing base (8523fa2) to head (26c8dde).

Files with missing lines	Patch %	Lines
assume/strategies/flexable.py	0.00%	3 Missing ⚠️
assume/strategies/learning_strategies.py	60.00%	2 Missing ⚠️
assume/units/powerplant.py	96.42%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #696      +/-   ##
==========================================
+ Coverage   81.99%   82.17%   +0.17%     
==========================================
  Files          56       56              
  Lines        9094     9089       -5     
==========================================
+ Hits         7457     7469      +12     
+ Misses       1637     1620      -17     

Flag	Coverage Δ
pytest	82.17% <86.95%> (+0.17%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 26c8dde)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 695 - Partially compliant Compliant requirements: Ensure downtime_warm_start is used when selecting start-up cost tier. (Context) Ramping constraints should keep working. Non-compliant requirements: Use get_starting_costs in energy_generation_costs / generation cost calculation. Requires further human verification: Ensure start-up cost is respected (incl. cold start in DMAS context). Confirm the intended semantics for start-up cost at the first simulation timestep (initial state), especially when a unit is initially off. Verify all affected strategies/paths now rely on the booked cost series (and don’t miss or double-count start-up costs) in real multi-market runs.

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue get_starting_costs and get_max_lookback_optime treat op_time/downtime thresholds as “index steps”, but the configuration variables (downtime_hot_start, downtime_warm_start) are commonly understood/validated elsewhere as hours. If the simulation index frequency is not 1 hour (e.g., 15 minutes), comparing “steps” to “hours” will select the wrong hot/warm/cold tier and compute incorrect start-up costs. def get_starting_costs(self, op_time: int) -> float: """ Returns the start-up cost for the given operation time. If ``op_time`` is positive, the unit is running and no start-up cost is returned. Otherwise the cost is selected from the hot / warm / cold tiers based on how many index steps the unit has been shut down. Args: op_time: The operation time, in index steps. Positive if running, negative (as produced by :meth:`get_operation_time`) if it was shut down. Returns: The start-up cost applicable to the given operation time. """ if op_time > 0: return 0 downtime = abs(op_time) if downtime <= self.downtime_hot_start: return self.hot_start_cost if downtime <= self.downtime_warm_start: return self.warm_start_cost return self.cold_start_cost Behavioral Change get_operation_time now assumes units are running at the start of the simulation (returning a positive operation time). This can suppress start-up cost booking at the first timestep for units that should be initially off but dispatch to on immediately, and it can affect ramp/min-up/min-down logic at the boundary. The lookback window covers the longest of the minimum operating time and the minimum down time (or the warm-start downtime threshold if the subclass defines one), so the value is meaningful both for ramping decisions and for tiering start-up costs (hot / warm / cold). Args: start (datetime.datetime): The start time. Returns: int: The operation time as a positive integer if operating, or negative if shut down. """ max_time = self.get_max_lookback_optime() begin = max(start - self.index.freq * max_time, self.index[0]) end = start - self.index.freq if start <= self.index[0]: # this assumes that all powerplants are running at the start of the simulation return max_time Behavioral Change Defaults for min_operating_time and min_down_time were changed from 0 to 1 for SupportsMinMax and SupportsMinMaxCharge. Units that previously had “no constraint unless configured” may now be constrained by default, changing dispatch feasibility and operation-time calculations when these values are omitted. min_operating_time: int = 1 min_down_time: int = 1

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 26c8dde
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Clamp time window to index Resetting start to the beginning of the simulation when it’s not an exact index label can silently overwrite start-cost history outside the requested window. Clamp start (and ideally end) to the nearest valid index boundary instead, and early-return if the window is fully outside self.index. This prevents incorrect re-booking of start costs when callers pass non-aligned timestamps. assume/units/powerplant.py [255-256] -if start not in self.index: +# Clamp window to the unit index to avoid overwriting outside the requested range +if start < self.index[0]: start = self.index[0] +elif start > self.index[-1]: + return +else: + start = self.index[self.index.get_indexer([start], method="bfill")[0]] Suggestion importance[1-10]: 7 __ Why: In PowerPlant._book_start_costs, resetting a non-aligned start to self.index[0] can indeed cause start-cost overwrites far outside the caller’s intended window. Clamping (and optionally early-returning when outside the index) improves correctness under non-aligned timestamps and multi-caller usage.	Medium
	Align costs to product timestamps Slicing by [first:last] can return a different length than products_index (missing timestamps, non-aligned boundaries), which can break profit -= costs or silently misalign costs. Reindex both series to products_index with fill_value=0.0 and convert to numpy arrays to guarantee correct shape and alignment. This makes reward computation robust under multi-market windows and sparse outputs. assume/strategies/flexable.py [218-225] -# TODO not available yet as generation costs are calculated afterwards -generation_costs = unit.outputs[f"{product_type}_generation_costs"].loc[ - products_index[0] : products_index[-1] -] -start_costs = unit.outputs[f"{product_type}_start_costs"].loc[ - products_index[0] : products_index[-1] -] -costs = np.asarray(generation_costs) + np.asarray(start_costs) +# Costs must align 1:1 with products_index +generation_costs = ( + unit.outputs[f"{product_type}_generation_costs"] + .reindex(products_index, fill_value=0.0) + .to_numpy(dtype=float) +) +start_costs = ( + unit.outputs[f"{product_type}_start_costs"] + .reindex(products_index, fill_value=0.0) + .to_numpy(dtype=float) +) +costs = generation_costs + start_costs Suggestion importance[1-10]: 6 __ Why: The current .loc[first:last] slices for *_generation_costs/*_start_costs can produce arrays that don’t align 1:1 with products_index, risking shape mismatch or silent misalignment in profit -= costs. Reindexing to products_index with fill_value=0.0 makes the reward computation more robust.	Low
	Avoid KeyError on missing costs Using .at[start] will raise a KeyError if start is not an exact index label or if start-costs weren’t booked for that timestamp yet. Read via .get(start, 0.0) (and coerce NaNs to 0.0) to avoid hard crashes during training runs. Apply the same pattern to other new reads of *_start_costs. assume/strategies/learning_strategies.py [634] -operational_cost += unit.outputs[f"{product_type}_start_costs"].at[start] +start_cost = float(unit.outputs[f"{product_type}_start_costs"].get(start, 0.0) or 0.0) +operational_cost += start_cost Suggestion importance[1-10]: 3 __ Why: Switching from .at[start] to Series.get(start, 0.0) would prevent KeyError if start isn’t an exact label, but in normal flow start is typically aligned to unit.index. Also, the proposed or 0.0 does not actually coerce NaN to 0.0, so it only partially fulfills the stated robustness goal.	Low

---

Previous suggestions

Suggestions up to commit fcc15cf

Category	Suggestion	Impact
Possible issue	Snap window start to index Falling back to self.index[0] when start is not exactly on the index can overwrite start-costs outside the requested window and break idempotency for partial-range calls. Instead, clamp to the simulation range and snap start to the next valid index timestamp (bfill), and return early if the window is fully out of range. assume/units/powerplant.py [255-256] -if start not in self.index: +if end < self.index[0] or start > self.index[-1]: + return + +if start < self.index[0]: start = self.index[0] +if start not in self.index: + start = self.index[self.index.get_indexer([start], method="bfill")[0]] + Suggestion importance[1-10]: 8 __ Why: The current fallback (start = self.index[0]) can unintentionally expand the booking window backwards, overwriting outputs[f"{product_type}_start_costs"] outside the requested range and breaking partial-window idempotency. Snapping start forward onto the index (and early-returning when out of range) is a correctness fix for non-aligned start timestamps.	Medium
	Align costs to product index Slice-based .loc[a:b] can return a different number of rows than products_index (e.g., if the unit index is 15min but products are hourly), causing shape mismatches or misaligned costs. Reindex both series to products_index (and fill NaNs) before converting to numpy so profit -= costs is always well-defined and correctly aligned. assume/strategies/flexable.py [219-225] -generation_costs = unit.outputs[f"{product_type}_generation_costs"].loc[ - products_index[0] : products_index[-1] -] -start_costs = unit.outputs[f"{product_type}_start_costs"].loc[ - products_index[0] : products_index[-1] -] -costs = np.asarray(generation_costs) + np.asarray(start_costs) +generation_costs = ( + unit.outputs[f"{product_type}_generation_costs"] + .reindex(products_index) + .fillna(0.0) + .to_numpy() +) +start_costs = ( + unit.outputs[f"{product_type}_start_costs"] + .reindex(products_index) + .fillna(0.0) + .to_numpy() +) +costs = generation_costs + start_costs Suggestion importance[1-10]: 6 __ Why: Replacing the .loc[a:b] slicing with .reindex(products_index) makes generation_costs/start_costs reliably aligned to products_index, avoiding length/shape mismatches when indices differ or have gaps. This is a solid robustness improvement around profit -= costs, though it may not affect cases where the indices already match perfectly.	Low
	Zero start-costs for storages Since downstream code and dashboards now expect {product_type}_start_costs, storages should actively overwrite that series to 0.0 for the window to avoid persisting stale values/NaNs across ticks. This keeps reporting consistent and prevents accidental carry-over when the same outputs frame is reused. assume/units/storage.py [310-314] # Overwrite the variable generation cost series from the finalized -# dispatch. Storages have no start-up costs, so no start-cost booking -# is needed. The helper uses `=` semantics so it is safe to call -# repeatedly as different markets clear for the same window. +# dispatch. Storages have no start-up costs, so ensure start-costs are 0. +start_cost_key = "energy_start_costs" +self.outputs[start_cost_key].loc[start:end] = 0.0 self.calculate_generation_cost(start, end, "energy") Suggestion importance[1-10]: 5 __ Why: If energy_start_costs exists in outputs, explicitly overwriting it to 0.0 for the window prevents stale values/NaNs from leaking into reporting now that dashboards and reward code reference {product_type}_start_costs. The change is small and likely safe here (storages have no start-up costs), but it’s more of a consistency/robustness improvement than a critical fix.	Low

[github-actions]

Persistent review updated to latest commit 26c8dde

[mthede]

first I thought this was a good assumption, but I'm not sure anymore because it doesn't account for the initial start-up costs for this ... but at least it's now clearly documented