Updates related to demand_tech in SLC framework#784
Draft
elenya-grant wants to merge 4 commits into
Draft
Conversation
…d only connect upstream techs to the controller
genevievestarke
left a comment
Collaborator
There was a problem hiding this comment.
I really like this functionality, thank you @elenya-grant !! I think it's much clearer what's happening in the code.
I don't have a problem with called the input demand_component since it needs to be a demand module to use system level control.
I don't think that we should throw an error for having upstream demand components in the system as long as it doesn't change the behavior of the system level control. I'm still working through why you would want a second demand component (so you can see how much of a steady-state demand you've met, maybe?).
It looks good to me!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates related to
demand_techin SLC frameworkREADY FOR HIGH-LEVEL REVIEW
This PR aims to resolve two issues related to SLC:
control_parameters#739In this draft implementation, I've required that the user specify the demand technology in the
system_level_controlsection of the plant configuration file. This looks like below:Background and Questions for reviewers on the above implementation:
An alternative approach would be have the
demand_componentdefined under thecontrol_parameterssection. The reason I did not do that approach is to be consistent with the current organization, usage, and distinction betweenslc_configandcontrol_parameters. The SLC gets two "configuration" dictionaries,slc_configandcontrol_parameters.slc_configis created inH2IntegrateModel._classify_slc_technologies(). Theslc_configcontains information that is used by both H2IntegrateModel and the SLC for set-up and connection logic. The parameters within thecontrol_parametersdictionary are controller-specific parameters that is only accessed within the SLC (not used by H2IntegrateModel). This is my justification for the route I went, but I'd be curious to hear opinions on the alternative approach if a reviewer finds it appealing.This simplifies some of the logic in
H2IntegrateModel_classify_slc_technologies(). I've also updated it so that only technologies that are upstream and connected to the demand technology are included in the information contained in theslc_config.Additional Questions for reviewers are noted in Section 2
Example use-cases that would be enabled with this functionality:
Section 1: Type of Contribution
Section 2: Draft PR Checklist
TODO:
Type of Reviewer Feedback Requested (on Draft PR)
Structural feedback:
Implementation feedback:
demand_componentparameter?demand_componenttechnology or should that be OK? Like if we had a hydrogen demand component and an ammonia demand component, using the ammonia demand as thedemand_component. I don't think we should raise an error, open to ideas though!Other feedback:
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 4: Related Issues
Resolves Issue #739 and #740
TODO: make an issue for the possible future use-case of having multiple demand components feed into the SLC (or even if thats a use-case worth considering)
Section 5: Impacted Areas of the Software
Section 5.1: New Files
h2integrate/core/test/test_slc_framework.py: added in tests forH2Integrate._classify_slc_technologies()h2integrate/core/test/inputs/tech_connection_cases.yaml: input file for tests inh2integrate/core/test/test_slc_framework.pySection 5.2: Modified Files
H2IntegrateModelinh2integrate/core/h2integrate_model.py__init__(): Updated call tocreate_technology_graph()create_technology_graph(): updated to take an input list of tech connections and return the graph (rather than set it as an attribute) so it can be used in_classify_slc_technologies()_classify_slc_technologies():demand_componentspecified in thesystem_level_controlconfig instead of looping throughtechnology_interconnections. This included adding in checking for the following cases where an error is raised:demand_techis missing from thesystem_level_controlsection in the plant configdemand_techis missing from the technology configdemand_techis not connected intechnology_interconnectionsdemand_techis not a valid demand technology type (i.e., the performance model name does not include "DemandComponent")demand_techare included in thetech_to_commodityandtechnology_graphparameters of the SLC config.examples/35_system_level_control/*/plant_config.yaml: added indemand_componenttosystem_level_controlsection.Section 6: Additional Supporting Information
Section 7: Test Results, if applicable