Feature/add ct #189

paulf81 · 2025-12-18T17:16:04Z

This PR adds a first NG Combustion Turbine model: CombustionTurbineSimple to Hercules. I'll update this PR as we go but the model is in now and a first example but still to go are:

Add tests
Update documentation
@genevievestarke could you review this model and compare with what is in HOPP/H2I?

Note, some of the parameterization (minimum stable load, ramp rates, 1-hour start-up/shut down adapted a bit from: https://docs.nrel.gov/docs/fy24osti/87554.pdf, but only loosely, thinking mainly of Figure 2 and Table 1 and only taking the more physical parameters and not things like repair times etc.

Copilot

Pull request overview

This PR introduces a new natural gas combustion turbine model (CombustionTurbineSimple) to Hercules with basic state management, ramp rate constraints, minimum stable load enforcement, and fuel consumption tracking. The implementation includes integration with the plant component system, a basic test, and a comprehensive example demonstrating turbine operation through various power setpoints and state transitions.

Key Changes:

Adds CombustionTurbineSimple component with 4-state machine (off, starting, on, stopping) and configurable operational constraints
Integrates combustion turbine as a generator component in the utilities and hybrid plant infrastructure
Provides Example 07 demonstrating a 6-hour simulation with complex control schedule

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`hercules/plant_components/combustion_turbine_simple.py`	New combustion turbine model with state machine, ramp constraints, fuel consumption, and part-load efficiency calculations
`hercules/hybrid_plant.py`	Adds component instantiation logic for CombustionTurbineSimple
`hercules/utilities.py`	Registers combustion_turbine as available component and generator type
`tests/combustion_turbine_simple_test.py`	Basic initialization test (requires missing test fixture to run)
`examples/07_ngct/hercules_input.yaml`	Configuration for 100 MW turbine with 20% minimum load
`examples/07_ngct/hercules_runscript.py`	Controller implementing multi-stage power schedule
`examples/07_ngct/plot_outputs.py`	Visualization script for power, state, and fuel consumption
`examples/07_ngct/README.md`	Documentation of example configuration and control schedule

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

Copilot · 2025-12-18T17:22:36Z

hercules/plant_components/combustion_turbine_simple.py

+                self.time_in_state = 0.0
+                return 0.0
+
+            return shutdown_power


The _control method uses if-elif statements for all state checks but doesn't have a final else clause or explicit return at the end of the function. If self.state_num has an invalid value (though validated in init), the function will implicitly return None. For robustness and clarity, add an else clause that raises an exception for unexpected state values.

Suggested change

return shutdown_power

return shutdown_power

else:

raise ValueError(f"Unexpected state_num in _control: {self.state_num}")

Copilot · 2025-12-18T17:22:36Z

hercules/plant_components/combustion_turbine_simple.py

+        if self.rated_capacity <= 0:
+            raise ValueError("rated_capacity must be greater than 0")
+        if self.min_stable_load_fraction < 0 or self.min_stable_load_fraction > 1:
+            raise ValueError("min_stable_load_fraction must be between 0 and 1 (inclusive)")


The error message references 'min_stable_load_fraction' but the parameter in the h_dict and documentation is called 'min_stable_load'. This inconsistency between the internal variable name and the user-facing parameter name could confuse users. Consider using 'min_stable_load' in the error message to match the documented parameter name.

Suggested change

raise ValueError("min_stable_load_fraction must be between 0 and 1 (inclusive)")

raise ValueError("min_stable_load must be between 0 and 1 (inclusive)")

I think min_stable_load_fraction is more informative, moving everything to that

Copilot · 2025-12-18T17:22:37Z

hercules/plant_components/combustion_turbine_simple.py

+            dict: Updated h_dict with combustion turbine outputs:
+                - power: Actual power output [kW]
+                - state_num: Operating state number (0=off, 1=starting, 2=on, 3=stopping)
+                - state_name: Operating state string ("off", "starting", "on", "stopping")


The step method documentation lists 'state_name' as an output (line 189), but the actual code has this output commented out (line 208). This creates a discrepancy between documentation and implementation. Either uncomment the output if it should be included, or remove it from the documentation if it's intentionally excluded.

Suggested change

- state_name: Operating state string ("off", "starting", "on", "stopping")

This output was removed because it created problems to have a string in the h_dict output, (and anyway the info is in statenum). Removing from docstring

Copilot · 2025-12-18T17:22:37Z

tests/combustion_turbine_simple_test.py

+import copy
+
+from hercules.plant_components.combustion_turbine_simple import CombustionTurbineSimple
+
+from .test_inputs.h_dict import (
+    h_dict_combustion_turbine,
+)
+
+
+def test_init_from_dict():
+    """Test that CombustionTurbineSimple can be initialized from a dictionary."""
+    ct = CombustionTurbineSimple(copy.deepcopy(h_dict_combustion_turbine))
+    assert ct is not None


The test suite lacks coverage for core combustion turbine functionality. Consider adding tests for: 1) state transitions (OFF→STARTING→ON→STOPPING→OFF), 2) minimum stable load enforcement, 3) ramp rate constraints, 4) min_up_time and min_down_time constraints, 5) fuel consumption calculations, 6) part load efficiency degradation, and 7) startup/shutdown time behavior. These are critical features that should have test coverage given that other components in the codebase have comprehensive tests.

This is in TODO

Copilot · 2025-12-18T17:22:37Z

hercules/plant_components/combustion_turbine_simple.py

+
+        # Check that initial conditions are valid
+        if self.power_output < 0 or self.power_output > self.rated_capacity:
+            raise ValueError("power_output must be between 0 and rated_capacity (inclusive)")


The error message references 'power_output' but this is not a user-provided parameter - it comes from initial_conditions["power"]. The error message should reference 'initial power' or 'initial_conditions.power' to help users identify which configuration value is invalid.

Suggested change

raise ValueError("power_output must be between 0 and rated_capacity (inclusive)")

raise ValueError(

"initial_conditions['power'] (initial power) must be between 0 and rated_capacity (inclusive)"

)

Copilot · 2025-12-18T17:22:38Z

hercules/plant_components/combustion_turbine_simple.py

+            raise ValueError(
+                "state_num must be one of the following: " + str(self.STATE_NAMES.values())
+            )


The error message on line 120 calls str(self.STATE_NAMES.values()) which will produce an unhelpful output like "dict_values(['off', 'starting', 'on', 'stopping'])". Consider using a clearer format such as "state_num must be 0 (off), 1 (starting), 2 (on), or 3 (stopping)" or listing the valid integer values explicitly.

Suggested change

raise ValueError(

"state_num must be one of the following: " + str(self.STATE_NAMES.values())

)

valid_states = ", ".join(

f"{state} ({name})" for state, name in self.STATE_NAMES.items()

)

raise ValueError(f"state_num must be one of the following: {valid_states}")

Good suggestion, adopting it

Copilot · 2025-12-18T17:22:38Z

examples/07_ngct/hercules_runscript.py

+    """Simple controller for combustion turbine on/off scheduling.
+
+    This controller turns the combustion turbine on at 60 minutes,
+    runs at full power for 3 hours, then shuts down at 240 minutes.


The class docstring description is inconsistent with the actual controller behavior. The docstring says "runs at full power for 3 hours" but the implementation has a more complex schedule with multiple power level changes (100% → 50% → 10% → 100% → off) as documented in lines 5-9 of the module docstring. Consider updating the class docstring to accurately reflect the control logic or simply reference the module docstring.

Suggested change

"""Simple controller for combustion turbine on/off scheduling.

This controller turns the combustion turbine on at 60 minutes,

runs at full power for 3 hours, then shuts down at 240 minutes.

"""Controller implementing the NGCT schedule described in the module docstring.

The turbine starts off, then:

- At 60 minutes, it is commanded to 100% of rated capacity.

- At 120 minutes, it is reduced to 50% of rated capacity.

- At 180 minutes, it is reduced to 10% of rated capacity.

- At 210 minutes, it is increased back to 100% of rated capacity.

- At 240 minutes, it is commanded off.

Copilot · 2025-12-18T17:22:38Z

tests/combustion_turbine_simple_test.py

+from .test_inputs.h_dict import (
+    h_dict_combustion_turbine,
+)
+
+


The imported h_dict_combustion_turbine is not defined in the tests/test_inputs/h_dict.py file. This will cause an ImportError when the test is run. You need to add a combustion turbine h_dict fixture to tests/test_inputs/h_dict.py following the pattern of other components like h_dict_electrolyzer or h_dict_battery.

Suggested change

from .test_inputs.h_dict import (

h_dict_combustion_turbine,

)

# Local combustion turbine h_dict fixture used for initializing CombustionTurbineSimple

h_dict_combustion_turbine = {

"name": "simple_combustion_turbine",

}

Copilot · 2025-12-18T17:22:38Z

tests/combustion_turbine_simple_test.py

+def test_init_from_dict():
+    """Test that CombustionTurbineSimple can be initialized from a dictionary."""
+    ct = CombustionTurbineSimple(copy.deepcopy(h_dict_combustion_turbine))
+    assert ct is not None


This test only verifies initialization succeeds but doesn't test any actual behavior or state of the combustion turbine. Consider adding assertions to verify key properties like rated_capacity, min_stable_load_fraction, state_num, and power_output match the expected values from the h_dict.

This is a TODO item

Copilot · 2025-12-18T17:22:39Z

hercules/plant_components/combustion_turbine_simple.py

+        if self.state_num not in self.STATE_NAMES:
+            raise ValueError(
+                "state_num must be one of the following: " + str(self.STATE_NAMES.values())
+            )


The state_num validation is redundant. Lines 116-117 already check if state_num is between 0 and 3 (inclusive), which is identical to checking if it's in STATE_NAMES (which contains keys 0, 1, 2, 3). The check on line 118 will always pass if line 116 passes. Consider removing lines 118-121 to eliminate redundancy.

Suggested change

if self.state_num not in self.STATE_NAMES:

raise ValueError(

"state_num must be one of the following: " + str(self.STATE_NAMES.values())

)

paulf81 · 2025-12-22T19:31:35Z

@misi9170 and @genevievestarke , update today is I merged back in develop to this PR and then went through the review comments from co-pilot (most of which were helpful!)

paulf81 added 5 commits December 17, 2025 16:08

Add combustion turbine model

3ccb8e3

add to hybrid plant

b7b4696

add to utilities

0f65a52

add first test

b9e8200

add new example

9eb9535

paulf81 requested review from Copilot, genevievestarke and misi9170 December 18, 2025 17:16

paulf81 self-assigned this Dec 18, 2025

Copilot started reviewing on behalf of paulf81 December 18, 2025 17:16 View session

Copilot AI reviewed Dec 18, 2025

View reviewed changes

paulf81 added 9 commits December 19, 2025 12:39

add test dicts

36d5493

Merge branch 'develop' into feature/add_ct

7cf9dca

address comment

24896c6

standardize to min_stable_load_fraction

1e386a1

address comment

cc58bd0

clean up state_num error checks

dca6e13

clean up step function

ce8d2c7

add else case

494538d

linting

1bc2961

paulf81 added 2 commits December 22, 2025 12:40

fix long line

d75f6e0

fix long line

eccc7a7

	raise ValueError("min_stable_load_fraction must be between 0 and 1 (inclusive)")
	raise ValueError("min_stable_load must be between 0 and 1 (inclusive)")

-            raise ValueError(
-                "state_num must be one of the following: " + str(self.STATE_NAMES.values())
-            )
+            valid_states = ", ".join(
+                f"{state} ({name})" for state, name in self.STATE_NAMES.items()
+            )
+            raise ValueError(f"state_num must be one of the following: {valid_states}")

-    """Simple controller for combustion turbine on/off scheduling.
-    This controller turns the combustion turbine on at 60 minutes,
-    runs at full power for 3 hours, then shuts down at 240 minutes.
+    """Controller implementing the NGCT schedule described in the module docstring.
+    The turbine starts off, then:
+    - At 60 minutes, it is commanded to 100% of rated capacity.
+    - At 120 minutes, it is reduced to 50% of rated capacity.
+    - At 180 minutes, it is reduced to 10% of rated capacity.
+    - At 210 minutes, it is increased back to 100% of rated capacity.
+    - At 240 minutes, it is commanded off.

-from .test_inputs.h_dict import (
-    h_dict_combustion_turbine,
-)
+# Local combustion turbine h_dict fixture used for initializing CombustionTurbineSimple
+h_dict_combustion_turbine = {
+    "name": "simple_combustion_turbine",
+}

Feature/add ct #189

Are you sure you want to change the base?

Feature/add ct #189

Uh oh!

Conversation

paulf81 commented Dec 18, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulf81 commented Dec 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant