Update Naming Convention in Open-Loop Converter Control Strategies#631
Conversation
|
"Would it make sense to add the outputs of the |
genevievestarke
left a comment
There was a problem hiding this comment.
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.
h2integrate/demand/generic_demand.py
Outdated
| 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 |
There was a problem hiding this comment.
Is this shared between demand_openloop_converter and flexible_demand_openloop_converter? Could we move these to the base class?
There was a problem hiding this comment.
No - the demand and flexible demand openloop "controllers" both shared the baseclass ConverterOpenLoopControlBase
There was a problem hiding this comment.
oh sorry - totally misunderstood your question. I think that these calculations could be moved to a method in the baseclass
There was a problem hiding this comment.
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.
Big fan!
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 That being said, I like the current definition of
Would it make sense to add a new folder/category? Something like If helpful, I'm happy to chat through this more on a call! |
|
@johnjasa @genevievestarke - thanks for the feedback! Quick question: I almost think |
jaredthomas68
left a comment
There was a problem hiding this comment.
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.
h2integrate/demand/generic_demand.py
Outdated
| 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 |
There was a problem hiding this comment.
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.
genevievestarke
left a comment
There was a problem hiding this comment.
I agree with making these demand components!
I like DemandComponentBase option for naming. Otherwise, I agree with all the naming suggestions!
h2integrate/demand/generic_demand.py
Outdated
| 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"] = ( |
There was a problem hiding this comment.
would commodity_set_point replace this commodity_out variable, or would there be both?
There was a problem hiding this comment.
there would only be commodity_out, which is equivalent to the commodity_out definition in the storage performance models.
h2integrate/demand/generic_demand.py
Outdated
| outputs[f"total_{self.commodity}_produced"] / self.fraction_of_year_simulated | ||
| ) | ||
|
|
||
| outputs["capacity_factor"] = ( |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
docs/control/control_overview.md
Outdated
| - [`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) |
There was a problem hiding this comment.
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 | |||
There was a problem hiding this comment.
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: | |||
There was a problem hiding this comment.
Does it still live under control_strategy in the tech_config?
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
I'd just search for the word control and remove it :)
docs/user_guide/model_overview.md
Outdated
| - 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 |
There was a problem hiding this comment.
not open-loop controllers?
There was a problem hiding this comment.
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 :)
h2integrate/demand/generic_demand.py
Outdated
|
|
||
| class DemandOpenLoopConverterController(ConverterOpenLoopControlBase): | ||
| class GenericDemandComponent(DemandComponentBase): | ||
| """Open-loop controller for converting input supply into met demand. |
There was a problem hiding this comment.
can we update docstring to not say controller? and the description of what it computes?
There was a problem hiding this comment.
I think I've updated all the doc strings and inline comments!
h2integrate/demand/generic_demand.py
Outdated
| * ``{commodity}_out``: Actual output delivered. | ||
| * ``unmet_{commodity}_demand_out``: Unmet demand. | ||
| * ``unused_{commodity}_out``: Curtailed production. | ||
| * ``{commodity}_set_point``: Actual output delivered. |
There was a problem hiding this comment.
You removed commodity_set_point right?
There was a problem hiding this comment.
removed it from the doc string!
h2integrate/demand/generic_demand.py
Outdated
| # 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"] = ( |
There was a problem hiding this comment.
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.
h2integrate/demand/generic_demand.py
Outdated
| # 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"] = ( |
There was a problem hiding this comment.
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.
docs/demand/demand_components.md
Outdated
| 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. |
There was a problem hiding this comment.
Converter Controller? --> Component?
docs/demand/demand_components.md
Outdated
| 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: |
There was a problem hiding this comment.
maybe remove "open-loop control framework"?
docs/demand/demand_components.md
Outdated
|
|
||
|
|
||
| #### Configuration | ||
| The flexible demand controller is defined within the `tech_config` with the following parameters: |
docs/demand/demand_components.md
Outdated
|
|
||
| ```yaml | ||
| model_inputs: | ||
| control_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"] |
| 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"] |
There was a problem hiding this comment.
Remove commented code? I'm assuming this is commented out because it moved to the base class?
h2integrate/demand/generic_demand.py
Outdated
| 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"] |
There was a problem hiding this comment.
Moved to the base class so can be removed?
There was a problem hiding this comment.
removed all commented out code and updated the docs and doc-strings to remove the word "control"
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"f"total_{commodity}_unmet_demand"PerformanceModelBaseClassUpdate 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->DemandComponentBaseDemandOpenLoopConverterController->GenericDemandComponentFlexibleDemandOpenLoopConverterController->FlexibleDemandComponentopenloop_controller_base.py->demand_base.pydemand_openloop_converter_controller.py->generic_demand.pyflexible_demand_openloop_controller.py->flexible_demand.pyOther changes:
compute_outputs()method to the baseclass, which simplified thecompute()methods of the two demand components that inherit itQuestions for reviewers are outlined in Section 2. This is ready for a draft review.
Section 1: Type of Contribution
Section 2: Draft PR Checklist
TODO:
Type of Reviewer Feedback Requested (on Draft PR)
Previous feedback - no longer accepting responses
Would it make sense to add the outputs of the
PerformanceModelBaseclassto 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.What should
commodity_set_pointmean with respect to converter controllers? Right now, it's the total commodity out to meet demand (equivalent tocommodity_outfrom storage performance models). I almost think this should be removed as an output and usecommodity_outinstead. 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.commodity_set_pointas 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/convertersto 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->OpenLoopDemandComponentBaseORDemandComponentBaseDemandOpenLoopConverterController->GenericDemandComponentFlexibleDemandOpenLoopConverterController->FlexibleDemandComponentThoughts on file renaming:
openloop_controller_base.py->demand_base.pydemand_openloop_converter_controller.py->generic_demand.pyflexible_demand_openloop_controller.py->flexible_demand.pyBELOW IS FEEDBACK THAT IS NEWLY REQUESTED
Section 3: General PR Checklist
docs/files are up-to-date, or added when necessaryCHANGELOG.md"A complete thought. [PR XYZ]((https://github.com/NatLabRockies/H2Integrate/pull/XYZ)", where
XYZshould 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/toh2integrate/demand/Section 4.1: New Files
h2integrate/control/test/test_converter_control_strategies.py: new test file for the tests of theDemandOpenLoopConverterControllerandFlexibleDemandOpenLoopConverterControllerthat previously existed inh2integrate/control/test/test_openloop_controllers.py. This has been moved toh2integrate/demand/test/Section 4.2: Modified Files
h2integrate/control/control_strategies/converters/demand_openloop_converter_controller.pyDemandOpenLoopConverterController: renamed outputs, added performance model outputsh2integrate/control/control_strategies/converters/flexible_demand_openloop_controller.pyFlexibleDemandOpenLoopConverterController: renamed outputs, added performance model outputsh2integrate/control/control_strategies/converters/openloop_controller_base.pyConverterOpenLoopControlBase: removedtotal_commodity_unmet_demandfrom 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 theDemandOpenLoopStorageController, 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 theDemandOpenLoopStorageController- 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
DemandOpenLoopStorageControllerand theDemandOpenLoopStorageController. 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
docs/developer_guide/coding_guidelines.mdattrsclass to define theConfigto load in attributes for the modelBaseConfigorCostModelBaseConfiginitialize()method,setup()method,compute()methodCostModelBaseClasssupported_models.pycreate_financial_modelinh2integrate_model.pytest_all_examples.pydocs/user_guide/model_overview.mddocs/section<model_name>.mdis added to the_toc.yml