Skip to content

fix: map all HydroGen subtypes to Hydropower in generator_mapping.yaml#64

Closed
PabloBotinGP wants to merge 1 commit into
Sienna-Platform:mainfrom
PabloBotinGP:pb/fix-hydro-generator-mapping
Closed

fix: map all HydroGen subtypes to Hydropower in generator_mapping.yaml#64
PabloBotinGP wants to merge 1 commit into
Sienna-Platform:mainfrom
PabloBotinGP:pb/fix-hydro-generator-mapping

Conversation

@PabloBotinGP

Copy link
Copy Markdown
Contributor

PowerSystems v5 added two new hydro types under HydroGen: HydroTurbine and HydroPumpTurbine. The pump-turbine one has a different prime mover code (PS for pumped storage) instead of the usual hydro code (HY).

The way the code figures out what category a generator belongs to is by going up the type tree until it finds a match in the mapping file. Before this fix, there was no entry for any hydro type, so HydroPumpTurbine kept going up until it hit a generic fallback that mapped PS to Storage — which is obviously wrong. This made stack plots show bad data or break when the system had pump-turbines.

The Fix

I added one entry to the Hydropower section of deps/generator_mapping.yaml, tied to the HydroGen parent type with no prime mover restriction.

Since HydroGen sits higher in the type tree than the generic fallback, this entry gets matched first. That means all hydro subtypes — current and future — should be correctly labeled as Hydropower no matter what prime mover code they use.

Closes Sienna-Platform/PowerGraphics.jl#132

Tests pass and I did some manual verification but can you double check this fixes your issue @jarry7 ?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates generator categorization to ensure all HydroGen subtypes (including new PowerSystems v5 hydro pump-turbines using prime mover PS) are classified as Hydropower instead of incorrectly falling back to Storage.

Changes:

  • Add a HydroGen catch-all entry under the Hydropower category in deps/generator_mapping.yaml to short-circuit the type-tree fallback logic.
  • Preserve existing primemover: HY mapping while ensuring future hydro subtypes inherit Hydropower categorization regardless of prime mover.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

NG-Steam:
- {gentype: Any, primemover: ST, fuel: NATURAL_GAS}
Hydropower:
- {gentype: HydroGen, primemover: null, fuel: null}

Copilot AI Apr 13, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new HydroGen catch-all mapping changes categorization behavior (e.g., it should prevent primemover: PS hydro units from being classified as Storage), but the existing tests only assert category keys and a few component selections. Consider adding a regression test that exercises the mapping resolution for a HydroGen subtype with primemover=PS and asserts it categorizes as Hydropower (not Storage).

Copilot uses AI. Check for mistakes.
PowerSystems v5 introduced HydroPumpTurbine (primemover=PS), which previously
fell through the YAML resolver to the generic Storage rule. Add a HydroGen
catch-all rule before the Any/HY rule so any HydroGen subtype is categorized
as Hydropower regardless of prime mover.

Also adds a regression test covering HydroPumpTurbine and HydroTurbine.

Closes PowerGraphics.jl#132.
@PabloBotinGP PabloBotinGP force-pushed the pb/fix-hydro-generator-mapping branch from 6ff108f to 081e43b Compare May 7, 2026 18:01
@jd-lara

jd-lara commented May 18, 2026

Copy link
Copy Markdown
Member

closed in favor of #72

@jd-lara jd-lara closed this May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When creating plots (specifically stack plots), filtering to new hydro types errors

3 participants