Skip to content

Update Naming Convention in Open-Loop Converter Control Strategies#631

Merged
elenya-grant merged 27 commits intoNatLabRockies:developfrom
elenya-grant:dispatch/converter_ol_sync
Apr 11, 2026
Merged

Update Naming Convention in Open-Loop Converter Control Strategies#631
elenya-grant merged 27 commits intoNatLabRockies:developfrom
elenya-grant:dispatch/converter_ol_sync

Conversation

@elenya-grant
Copy link
Copy Markdown
Collaborator

@elenya-grant elenya-grant commented Mar 26, 2026

Update Naming Convention in Open-Loop Converter Control Strategies

NOTE: The intention of this PR is a step in resolving Issue #521. Eventually, the unmet demand and unused commodity calculations done in storage models would instead be done with the use of these demand components (see the follow-on PR, #666). Please keep this in mind when reviewing the new outputs of these models (please see this comment on how that is expected to change)

Update output naming convention in "open-loop converter control strategies" to align with the naming convention of the storage performance models.

  • f"{commodity}_unmet_demand" -> f"unmet_{commodity}_demand_out"
  • f"{commodity}_unused_commodity" -> f"unused_{commodity}_out"
  • Removed output f"total_{commodity}_unmet_demand"
  • Updated so that these inherit from the PerformanceModelBaseClass

Update as of 04/06/2026:

The above changes motivated a larger discussion around these "open-loop converter control strategies" and how they're actually used/what they were intended for. Some additional context on why these were originally put as "control strategies" is listed in Section 5. But - my motivating philosophy to renaming these classes and moving the files is that these components are not used as controllers and are instead ways of representing some generic demand signal. I think a common push-back on this idea is that demand should come from a converter component, but in some cases, we care more about the demand being met than what the demand is "turned into". For example (a common use-case), defining the energy demand of a data center is important so we can design a system to meet that demand and calculate the cost of energy to meet that demand. We are not trying to calculate the cost of the output from the data center, we only care about the inputs. These demand components are used to define a demand, subtract some initial amount from that demand, and output the remaining and excess demand. The unmet demand from the demand component can then be passed to some converter technology as the demand (example 23 is a good reflection of this). These models are primarily used to calculate demand metrics, not define a control strategy.

I propose the following renaming (already done):

  • ConverterOpenLoopControlBase -> DemandComponentBase
  • DemandOpenLoopConverterController -> GenericDemandComponent
  • FlexibleDemandOpenLoopConverterController -> FlexibleDemandComponent
  • openloop_controller_base.py -> demand_base.py
  • demand_openloop_converter_controller.py -> generic_demand.py
  • flexible_demand_openloop_controller.py -> flexible_demand.py

Other changes:

  • added compute_outputs() method to the baseclass, which simplified the compute() methods of the two demand components that inherit it
  • re-classified the demand component as a performance model instead of a control strategy

Questions for reviewers are outlined in Section 2. This is ready for a draft review.

Section 1: Type of Contribution

  • Feature Enhancement
    • Framework
    • New Model
    • Updated Model
    • Tools/Utilities
    • Other (please describe):
  • Bug Fix
  • Documentation Update
  • CI Changes
  • Other (please describe):

Section 2: Draft PR Checklist

  • Open draft PR
  • Describe the feature that will be added
  • Fill out TODO list steps
  • Describe requested feedback from reviewers on draft PR
  • Complete Section 7: New Model Checklist (if applicable)

TODO:

  • Remove commented out code
  • update doc pages

Type of Reviewer Feedback Requested (on Draft PR)

Previous feedback - no longer accepting responses

  • Would it make sense to add the outputs of the PerformanceModelBaseclass to these models? These models are more like ways to represent a generic end-use (or reference signal) rather than actual control strategies. So-if they represent an end-use and we may want to calculate the LCO of the end-use demand, then we'd need these to calculate/output rated commodity capacity and capacity factor.

    • I've integrated this for now - would love feedback on how some of the outputs are calculated.
  • What should commodity_set_point mean with respect to converter controllers? Right now, it's the total commodity out to meet demand (equivalent to commodity_out from storage performance models). I almost think this should be removed as an output and use commodity_out instead. If these are ever being used "as a controller" then it's probably the unmet demand that's being passed to some converter as the demand profile.

    • I plan to remove commodity_set_point as an output.
  • Since the converter openloop control strategies are more like ways to represent an end-use, should these models be moved from control/control_strategies/converters to a different folder (such as):

    • transporters/
    • transporters/demand/
    • converters/end_use/
    • demand/ (this is what I went with)
    • end_use/
  • Thoughts on proposed class re-naming:

    • ConverterOpenLoopControlBase -> OpenLoopDemandComponentBase OR DemandComponentBase
    • DemandOpenLoopConverterController -> GenericDemandComponent
    • FlexibleDemandOpenLoopConverterController -> FlexibleDemandComponent
  • Thoughts on file renaming:

    • openloop_controller_base.py -> demand_base.py
    • demand_openloop_converter_controller.py -> generic_demand.py
    • flexible_demand_openloop_controller.py -> flexible_demand.py

BELOW IS FEEDBACK THAT IS NEWLY REQUESTED

  • Any feedback on the output calculations (such as capacity factor) for the demand components?

Section 3: General PR Checklist

  • PR description thoroughly describes the new feature, bug fix, etc.
  • [-] Added tests for new functionality or bug fixes
  • Tests pass (If not, and this is expected, please elaborate in the Section 6: Test Results)
  • Documentation
    • Docstrings are up-to-date
    • Related docs/ files are up-to-date, or added when necessary
    • Documentation has been rebuilt successfully
    • Examples have been updated (if applicable)
  • CHANGELOG.md
    • At least one complete sentence has been provided to describe the changes made in this PR
    • After the above, a hyperlink has been provided to the PR using the following format:
      "A complete thought. [PR XYZ]((https://github.com/NatLabRockies/H2Integrate/pull/XYZ)", where
      XYZ should be replaced with the actual number.

Section 3: Related Issues

Section 4: Impacted Areas of the Software

Moved all the files from h2integrate/control/control_strategies/converters/ to h2integrate/demand/

Section 4.1: New Files

  • h2integrate/control/test/test_converter_control_strategies.py: new test file for the tests of the DemandOpenLoopConverterController and FlexibleDemandOpenLoopConverterController that previously existed in h2integrate/control/test/test_openloop_controllers.py. This has been moved to h2integrate/demand/test/

Section 4.2: Modified Files

  • h2integrate/control/control_strategies/converters/demand_openloop_converter_controller.py
    • DemandOpenLoopConverterController: renamed outputs, added performance model outputs
  • h2integrate/control/control_strategies/converters/flexible_demand_openloop_controller.py
    • FlexibleDemandOpenLoopConverterController: renamed outputs, added performance model outputs
  • h2integrate/control/control_strategies/converters/openloop_controller_base.py
    • ConverterOpenLoopControlBase: removed total_commodity_unmet_demand from output, updated output naming as outlined in Section 1.

Section 5: Additional Supporting Information

These control strategies were developed during the natural-gas for data-center work. When I initially made what is now the DemandOpenLoopConverterController - I called it a "generic demand component". This was brought into H2I shortly after the DemandOpenLoopStorageController, which, at the time, did similar calculations for unmet demand, excess commodity production, etc. Since the generic demand component had such similarities to the outputs of the DemandOpenLoopStorageController - it was decided to define this as a "converter control strategy".

However, since the work on refactoring the storage dispatch and storage performance models (Issue #564, which most relevantly included PR #566, #498, and #612), the storage controllers no longer output the excess commodity produced or unmet demand, these are done in the storage performance models.

At this stage, there is no similar usage between the DemandOpenLoopStorageController and the DemandOpenLoopStorageController. Therefore, I think that its time to refactor the names of these demand components so that they can be used to resolve Issue #521 and be defined based on their primary usage, which is to calculate demand-related metrics rather than function as a controller.

Section 6: Test Results, if applicable

Section 7 (Optional): New Model Checklist

  • Model Structure:
    • Follows established naming conventions outlined in docs/developer_guide/coding_guidelines.md
    • Used attrs class to define the Config to load in attributes for the model
      • If applicable: inherit from BaseConfig or CostModelBaseConfig
    • Added: initialize() method, setup() method, compute() method
      • If applicable: inherit from CostModelBaseClass
  • Integration: Model has been properly integrated into H2Integrate
    • Added to supported_models.py
    • If a new commodity_type is added, update create_financial_model in h2integrate_model.py
  • Tests: Unit tests have been added for the new model
    • Pytest-style unit tests
    • Unit tests are in a "test" folder within the folder a new model was added to
    • If applicable add integration tests
  • Example: If applicable, a working example demonstrating the new model has been created
    • Input file comments
    • Run file comments
    • Example has been tested and runs successfully in test_all_examples.py
  • Documentation:
    • Write docstrings using the Google style
    • Model added to the main models list in docs/user_guide/model_overview.md
      • Model documentation page added to the appropriate docs/ section
      • <model_name>.md is added to the _toc.yml

@elenya-grant elenya-grant added ready for review This PR is ready for input from folks dispatch related to dispatch and control labels Mar 27, 2026
@kbrunik
Copy link
Copy Markdown
Collaborator

kbrunik commented Mar 31, 2026

"Would it make sense to add the outputs of the PerformanceModelBaseclass to these models?" I think this would be great! and makes them more flexible/similar to other performance models.

Copy link
Copy Markdown
Collaborator

@genevievestarke genevievestarke left a comment

Choose a reason for hiding this comment

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

I like having this component inherit the PerformanceModelBaseclass! I think the ultimate goal is to make this its own component and do all the demand-based calculations here. This would move enable these calculations for converter systems, but also move them out of the storage performance models, which will help untangle the system/storage question.
For the second point (and third point), I think that this should be reclassified as a transporter, so that it can output unmet demand (and not a set point). This makes sense to me because it would be used for storage and converters, and it is more of an analysis component (like a combiner) than a control component.

outputs[f"unmet_{self.commodity}_demand_out"] = np.where(
remaining_demand > 0, remaining_demand, 0
)
outputs[f"unused_{self.commodity}_out"] = np.where(
remaining_demand < 0, -1 * remaining_demand, 0
)

# Calculate actual output based on demand met and curtailment
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this shared between demand_openloop_converter and flexible_demand_openloop_converter? Could we move these to the base class?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No - the demand and flexible demand openloop "controllers" both shared the baseclass ConverterOpenLoopControlBase

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

oh sorry - totally misunderstood your question. I think that these calculations could be moved to a method in the baseclass

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree with including set_point, but most of the below calculations don't make sense to me in a controller. I could see it making sense to have a simple converter or end_use component that goes along with the simplistic controller here.

@johnjasa
Copy link
Copy Markdown
Collaborator

johnjasa commented Apr 1, 2026

Would it make sense to add the outputs of the PerformanceModelBaseclass to these models? These models are more like ways to represent a generic end-use (or reference signal) rather than actual control strategies. So-if they represent an end-use and we may want to calculate the LCO of the end-use demand, then we'd need these to calculate/output rated commodity capacity and capacity factor.
I've integrated this for now - would love feedback on how some of the outputs are calculated.

Big fan!

What should commodity_set_point mean with respect to converter controllers? Right now, it's the total commodity out to meet demand (equivalent to commodity_out from storage performance models)

You asking this question made me dig deeper into how I'm viewing these classes, and I realized I might have some potentially conflicting viewpoints. Mainly, this isn't a converter in the way that I initially envisioned them. Specifically, it's not actually converting a commodity, but more tracking it. So I appreciate your question, but I think I agree with @genevievestarke that set_point makes less intuitive sense to me than it could.

That being said, I like the current definition of commodity_set_point being the total commodity to meet demand.

Since the converter openloop control strategies are more like ways to represent an end-use, should these models be moved from control/control_strategies/converters to a different folder (such as):
transporters/
transporters/demand/
converters/end_use/

Would it make sense to add a new folder/category? Something like end_uses at the same level as transporters and converters? I ask that because this is essentially a new area that we might have multiple components joining soon. I was previously thinking things like this would fall under converters (e.g. view a data center as converting electricity into compute) but I'm not necessarily convinced by that organization.

If helpful, I'm happy to chat through this more on a call!

@elenya-grant
Copy link
Copy Markdown
Collaborator Author

@johnjasa @genevievestarke - thanks for the feedback! Quick question:

I almost think commodity_set_point should be removed as an output and we could use commodity_out instead. If these are ever being used "as a controller" then its probably the unmet demand that's being passed to some converter as the demand profile. What are yalls thoughts about that option?

Copy link
Copy Markdown
Collaborator

@jaredthomas68 jaredthomas68 left a comment

Choose a reason for hiding this comment

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

I'm not sure I fully understand your plan. As I see it, these components are a mix of control and performance and the two elements should be separated more clearly, not integrated more closely.

I do find the idea of end_use components interesting. I had previously been seeing those as converter that take in resources and output money, or something along those line, but since we are using finance models throughout, the money side is already handled and the end use starts to make more sense to me.

outputs[f"unmet_{self.commodity}_demand_out"] = np.where(
remaining_demand > 0, remaining_demand, 0
)
outputs[f"unused_{self.commodity}_out"] = np.where(
remaining_demand < 0, -1 * remaining_demand, 0
)

# Calculate actual output based on demand met and curtailment
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree with including set_point, but most of the below calculations don't make sense to me in a controller. I could see it making sense to have a simple converter or end_use component that goes along with the simplistic controller here.

Copy link
Copy Markdown
Collaborator

@genevievestarke genevievestarke left a comment

Choose a reason for hiding this comment

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

I agree with making these demand components!
I like DemandComponentBase option for naming. Otherwise, I agree with all the naming suggestions!

remaining_demand < 0, -1 * remaining_demand, 0
)

# Calculate actual output based on demand met and curtailment
outputs[f"{commodity}_set_point"] = (
inputs[f"{commodity}_in"] - outputs[f"{commodity}_unused_commodity"]
outputs[f"{self.commodity}_out"] = (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

would commodity_set_point replace this commodity_out variable, or would there be both?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

there would only be commodity_out, which is equivalent to the commodity_out definition in the storage performance models.

outputs[f"total_{self.commodity}_produced"] / self.fraction_of_year_simulated
)

outputs["capacity_factor"] = (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think that there are more useful outputs we can add rather than the capacity factor. I think things based on the demand would be more useful, like total_unmet_demand and total_demand_met, maybe the capacity factor of the demand, and basing them on demand rather than the rated capacity.

Is this necessary because it's inheriting the performance base class?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The capacity factor is the capacity factor of the demand (total_demand_met/total_demand).
I heard you about those outputs - but I don't see what those outputs would be connected to and they could be easily calculated from the timeseries profiles. So - I think we should add those outputs once they're needed.

@elenya-grant elenya-grant marked this pull request as ready for review April 7, 2026 22:49
- [`DemandOpenLoopStorageController`](#demand-open-loop-storage-controller)
- [`DemandOpenLoopConverterController`](#demand-open-loop-converter-controller)
- [`FlexibleDemandOpenLoopConverterController`](#flexible-demand-open-loop-converter-controller)
- [`GenericDemandComponent`](#demand-open-loop-converter-controller)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would make sense to move the GenericDemandComponent and FlexibleDemandComponent out of the controls portion of the docs and maybe adding it to a new folder called demand?

@@ -37,7 +37,7 @@ This page documents two core controller types:

(demand-open-loop-converter-controller)=
### Demand Open-Loop Converter Controller
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like this heading could be updated to the new naming

@@ -57,19 +57,19 @@ The controller is defined within the `tech_config` and requires these inputs.

```yaml
control_strategy:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does it still live under control_strategy in the tech_config?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

not anymore!

I pulled out the "converter control stuff" from the control_overview.md page and put it in a new page (demand_components). I still need to update the model_overview.md page and the language in the demand_components.md file.

demand_profile: [10, 10, 12, 15, 14]
```
For an example of how to use the `DemandOpenLoopConverterController` open-loop control framework, see the following:
For an example of how to use the `GenericDemandComponent` open-loop control framework, see the following:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd just search for the word control and remove it :)

- Converter Controllers:
- `'DemandOpenLoopConverterController'`: open-loop control; manages resource flow based on demand constraints
- `'FlexibleDemandOpenLoopConverterController'`: open-loop control; manages resource flow based on demand and flexibility constraints
- `'GenericDemandComponent'`: open-loop control; manages resource flow based on demand constraints
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not open-loop controllers?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like how these were moved to a demand folder and renamed. It makes more sense after all of the dispatch refactor. Thanks @elenya-grant :)


class DemandOpenLoopConverterController(ConverterOpenLoopControlBase):
class GenericDemandComponent(DemandComponentBase):
"""Open-loop controller for converting input supply into met demand.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we update docstring to not say controller? and the description of what it computes?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think I've updated all the doc strings and inline comments!

* ``{commodity}_out``: Actual output delivered.
* ``unmet_{commodity}_demand_out``: Unmet demand.
* ``unused_{commodity}_out``: Curtailed production.
* ``{commodity}_set_point``: Actual output delivered.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You removed commodity_set_point right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

removed it from the doc string!

# Calculate actual output based on demand met and curtailment
outputs[f"{commodity}_set_point"] = (
inputs[f"{commodity}_in"] - outputs[f"{commodity}_unused_commodity"]
outputs[f"{self.commodity}_out"] = (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It probably would be helpful to add a description of what commodity_out means in the case of the demand to the docs since it's not particularly intuitive.

# Calculate actual output based on demand met and curtailment
outputs[f"{commodity}_set_point"] = (
inputs[f"{commodity}_in"] - outputs[f"{commodity}_unused_commodity"]
outputs[f"{self.commodity}_out"] = (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think I'll add a figure to the docs explaining some of these outputs. And a note about how the CF will be less than one and the edge case about how to account for that.

@elenya-grant elenya-grant requested a review from kbrunik April 10, 2026 18:01
Copy link
Copy Markdown
Collaborator

@kbrunik kbrunik left a comment

Choose a reason for hiding this comment

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

Just a few nitpicks but I think this looks great! And thank you for creating issue #667. I will tackle that next week

Demand components define rule-based logic for meeting commodity demand profiles without using dynamic system feedback. These components operate independently at each timestep.

This page documents two core controller types:
1. Demand Open-Loop Converter Controller — meets a fixed demand profile.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Converter Controller? --> Component?

commodity_units: kg/h
demand_profile: [10, 10, 12, 15, 14]
```
For an example of how to use the `GenericDemandComponent` open-loop control framework, see the following:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe remove "open-loop control framework"?



#### Configuration
The flexible demand controller is defined within the `tech_config` with the following parameters:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove controller?


```yaml
model_inputs:
control_parameters:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

performance_parameters?

commodity = self.config.commodity
remaining_demand = inputs[f"{commodity}_demand"] - inputs[f"{commodity}_in"]

# remaining_demand = inputs[f"{self.commodity}_demand"] - inputs[f"{self.commodity}_in"]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove commented code?

outputs[f"{commodity}_set_point"] = (
inputs[f"{commodity}_in"] - outputs[f"{commodity}_unused_commodity"]
)
# flexible_remaining_demand = flexible_demand_profile - inputs[f"{self.commodity}_in"]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove commented code? I'm assuming this is commented out because it moved to the base class?

outputs = self.calculate_outputs(
inputs[f"{self.commodity}_in"], inputs[f"{self.commodity}_demand"], outputs
)
# remaining_demand = inputs[f"{self.commodity}_demand"] - inputs[f"{self.commodity}_in"]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Moved to the base class so can be removed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

removed all commented out code and updated the docs and doc-strings to remove the word "control"

@elenya-grant elenya-grant enabled auto-merge (squash) April 10, 2026 20:52
Copy link
Copy Markdown
Collaborator

@genevievestarke genevievestarke left a comment

Choose a reason for hiding this comment

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

Yay!!

@elenya-grant elenya-grant merged commit f5c1fa6 into NatLabRockies:develop Apr 11, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dispatch related to dispatch and control ready for review This PR is ready for input from folks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants