Skip to content

Template existing planned storage - #149

Open
EllieKallmier wants to merge 3 commits into
mainfrom
template-existing-planned-storage
Open

EllieKallmier wants to merge 3 commits into
mainfrom
template-existing-planned-storage

Conversation

@EllieKallmier

Copy link
Copy Markdown
Member

Wraps up the existing/planned templating started in #143 by adding the storage_existing_planned table, and moves the technology/station-keyed merge into helpers.py, so new entrants and existing/planned storage share one function (sorry for not splitting out into smaller PR chunks whoops).

Storage properties (the ones templated in this PR) come from three places:

  • Per unit (IASR ID): capacity, storage capacity, commissioning date, closure year. Uses the same unit-keyed merge as generators.
  • Per battery technology (battery_properties): charge/discharge efficiency
  • Per PHES station (pumped_hydro_..._properties): round-trip efficiency, split into symmetric charge/discharge legs

Where the changes live:

src/ispypsa/templater/
├── existing_planned.py   ← storage orchestrator + battery/PHES split merge; exclude_unit_keys
├── helpers.py            ← _merge_category_keyed_properties (moved here, now shared);
│                           _apply_iasr_table_replacements takes a list of corrections
├── mappings.py           ← storage unit, battery and PHES property maps
├── new_entrants.py       ← uses the shared merge instead of its own _merge_properties
└── create_template.py    ← wires in storage_existing_planned
tests/
├── test_templater/test_existing_planned.py   ← storage merge + orchestrator tests
├── test_templater/test_helpers.py            ← shared merge tests (moved from new entrants)
└── test_cli/test_create_ispypsa_inputs_new_table_formats.py ← 111 storage rows on 7.5 cache

Some design choices to flag

  • PHES = storage that isn't a battery: Wivenhoe and Shoalhaven are labelled "Hydro" in the summary, and routed to storage because they appear in the PHES properties table. In _merge_storage_type_split_properties I've split by complement from battery rows, but we could instead manually edit the technology value for these units from the start. I just did this for the moment to avoid getting too into the hydro-phes weeds while it's still a bit in the air - I imagine we might make other hydro-related decisions that could change what the ideal templater output shape for those units. But am very open to thoughts+opinions.
  • Two merge function options: Unit-level property merges match unit names one-to-one; battery/PHES merges match many-to-one (technology- or station- level properties). Mostly the difference is in which fuzzy-matching helper gets used to align the keys but it seemed to me like more effort than worth it to try and condense down into one ubiquitous function. (See note below).
  • exclude_unit_keys: Added based on suggestion from Nick in review of Template existing and planned (ECAA) generators #143 to tighten scope of fuzzy-matching for the unit-level property merges. Property tables otherwise have both generator and storage units all listed in the same table so if there were ever a typo'd generator name that made it a closer match to a storage unit name that would create an incorrect mapping (potentially problematic and kinda annoying to locate if it happened I reckon).
  • Empty-frame guards removed (following Nick's review comment on Template existing and planned (ECAA) generators #143). I thought about it some more and tried a few things and ended up disagreeing with myself in favour of a less-engineered approach. On the assumption that inputs to the templater will almost always be (if not always) the parsed IASR tables (which shouldn't be empty), and keeping the loud failure if something functional would quietly work but with unhelpful outcomes (eg validating property tables before merging so we don't get quiet all-NaN columns).
  • Row order. Storage output lists batteries then PHES, not summary order. The schema only requires unique names so shouldn't be a problem but noting here just to be clear.

Some other data notes to flag - see #131:

  • Shoalhaven unit-level vs station-level storage hours disagree (unit-level used)
  • Tumut 3 stays in generators (interim) instead of attempting to split into pumping/non-pumping units for now...

Note on the category merge function and property maps:
I thought about adding a field to the mapping dicts that would indicate which 'category' column should be used to merge onto for different properties (see 'df_key_col' in '_merge_category_keyed_properties'), but in the end decided against. Because: those mappings are currently IASR-table facing only; they just describe the input table features important for the property merge. I didn't want to broaden that context to make those maps also describe the part-templated summary table. BUT as always open to persuasion/other preferences.

@EllieKallmier EllieKallmier added type: feature New feature or request module: templater Covers contents of `templater` module labels Sep 30, 2026
@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/ispypsa/templater/create_template.py 93.33% <100.00%> (+0.07%) ⬆️
src/ispypsa/templater/existing_planned.py 100.00% <100.00%> (ø)
src/ispypsa/templater/helpers.py 100.00% <100.00%> (ø)
src/ispypsa/templater/mappings.py 100.00% <100.00%> (ø)
src/ispypsa/templater/new_entrants.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: templater Covers contents of `templater` module type: feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant