-
Notifications
You must be signed in to change notification settings - Fork 27
Ard integration for wind farm combined performance and cost model #312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
kbrunik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is super exciting--our first international example and some really cool functionality with the full-stack integration! Thanks for the great PR @jaredthomas68! Only a few small things from my perspective.
| The intent of [Ard](https://github.com/WISDEM/Ard) is to be a modular, full-stack multi-disciplinary optimization tool for wind farms. By incorporating Ard in H2Integrate, we are able to draw on many wind technology models developed at NREL and other institutions without managing them or their connections in Ard. Models connected in Ard include many parts of [WISDEM](https://github.com/WISDEM/WISDEM), [FLORIS](https://github.com/NREL/floris), and [OptiWindNet](https://github.com/DTUWindEnergy/OptiWindNet). Ard also provides constraint functions and wind farm layout generation capabilities among other things. Because Ard has been developed in a modular way, you may extend Ard fairly easily to include other wind models of interest. | ||
|
|
||
| Ard is included in H2Integrate as an [OpenMDAO sub-model](https://openmdao.org/newdocs/versions/latest/features/building_blocks/components/submodel_comp.html), which means that Ard is treated as an OpenMDAO system within an OpenMDAO system. In this way, the user can run an independent wind farm optimization within Ard, or allow H2Integrate to manage the wind farm design variables directly. The drawback of including Ard as a sub-model is that N2 diagrams made from the H2Integrate problems will show Ard only as a single model, rather than showing all the subsystems within Ard. IF you wish to view an N2 diagram of Ard, you will need to use the Ard problem instead. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to show how a user would create an Ard problem instead of just the H2Integrate problem?
| - FinanceSE | ||
|
|
||
| ## Wind Resource | ||
| The wind resource capabilities of H2Integrate are not yet connected with Ard, so the user must provide a wind resource file directly to the Ard model inputs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this because of how the wind resource needs to be formatted? If yes, could you explain the necessary format of the file and what the attributes--or column--requirements are)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like you're using openmeteo, is this something that you're integrating @elenya-grant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes - PR #332
| The wind resource capabilities of H2Integrate are not yet connected with Ard, so the user must provide a wind resource file directly to the Ard model inputs. | ||
|
|
||
| ## Examples | ||
| For an example of using Ard in an H2Integrate model, see `examples/xx_wind_ard`. Note that Ard uses a combination of input files, including a [wind IO](https://github.com/IEAWindSystems/windIO) file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a list of the necessary input files for ARD, with maybe a one sentence description about them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, and pointing to the Ard docs where possible here would be quite helpful!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this file different from the NREL_Reference_5MW_126.csv? Could one of them be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am also confused on why either of these CSV files are included because the turbine Cp curve information appears to be in examples/xx_wind_ard/ard_inputs/windIO-plant_turbine_NREL-5.0MW-126m-RWT.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems like this has the same info as the NREL_Reference_5MW_126.csv and the power_thrust_table_ccblade_NREL-5p0-126-RWT.csv, is it necessary to have the information duplicated?
| modeling_options: | ||
| windIO_plant: !include ../ard_inputs/windio.yaml | ||
| layout: | ||
| N_turbines: 9 #65 # should be 65 turbines |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be changed to 65?
|
|
||
|
|
||
| @define | ||
| class WindPlantArdModelConfig(BaseConfig): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could have WindPlantArdModelConfig() inherit CostModelBaseClass because the cost model base class already inherits BaseConfig and then you'd be able to get rid of the lines 9-22 and still have access to the cost_year, but @elenya-grant or @johnjasa correct me if I'm wrong. Or maybe this is specific to the sub_model setup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked this over, I believe this WindArdCostComponent is necessary because the setup() method of the CostModelBaseClass assumes that you have created self.config. If we don't call that here in this inheriting class' setup() and just tried to use CostModelBaseClass directly, it would fail on the lack of self.config.
I don't think we need to change anything in the framework for the cost model baseclasses to behave differently as part of this PR. It did take me a sec to understand why this is here so I've added a note in the docstring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @johnjasa
| class WindPlantArdModelConfig(BaseConfig): | ||
| """Configuration container for Ard wind plant model inputs. | ||
| Attributes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we reformat this docstring to Google style?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
|
||
| class ArdWindPlantModel(om.Group): | ||
| """ | ||
| OpenMDAO Group integrating the Ard wind plant as a sub-problem.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reformat to Google style please and I see a double period on line 42.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and removed
| # Post-process the results | ||
| model.post_process() | ||
|
|
||
| with subtests.test("Check wind generation"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to not set the generation to a value? It seems like it might be easier to check in the future if this test breaks if we have a better idea of the value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Kaitlin - I think that having subtests on the value of the total wind generation profile and total solar generation profile would be a good so if other subtests fail (like LCOE or total electricity dispatched, etc), it will be easier to identify what causes other subtest failures.
johnjasa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool stuff! It's really fun to see different projects and tools get connected. I think this strengthens our group and its capabilities well.
I've pushed some very minor changes directly. I'm requesting changes, mostly for things that @kbrunik noted that I'd like to see addressed or discussed. Thank you!
| The wind resource capabilities of H2Integrate are not yet connected with Ard, so the user must provide a wind resource file directly to the Ard model inputs. | ||
|
|
||
| ## Examples | ||
| For an example of using Ard in an H2Integrate model, see `examples/xx_wind_ard`. Note that Ard uses a combination of input files, including a [wind IO](https://github.com/IEAWindSystems/windIO) file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, and pointing to the Ard docs where possible here would be quite helpful!
| resource_dir: "../11_hybrid_energy_plant/tech_inputs/weather/solar" | ||
| resource_filename: "30.6617_-101.7096_psmv3_60_2013.csv" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the re-use of the existing solar data, though it might be confusing for folks looking at this to see a different lat/lon location used. I'd suggest either adding a comment here noting that it's a notional location and being used generically, or adding a different weather file.
|
|
||
|
|
||
| @define | ||
| class WindPlantArdModelConfig(BaseConfig): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked this over, I believe this WindArdCostComponent is necessary because the setup() method of the CostModelBaseClass assumes that you have created self.config. If we don't call that here in this inheriting class' setup() and just tried to use CostModelBaseClass directly, it would fail on the lack of self.config.
I don't think we need to change anything in the framework for the cost model baseclasses to behave differently as part of this PR. It did take me a sec to understand why this is here so I've added a note in the docstring.
| class WindPlantArdModelConfig(BaseConfig): | ||
| """Configuration container for Ard wind plant model inputs. | ||
| Attributes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
|
||
| class ArdWindPlantModel(om.Group): | ||
| """ | ||
| OpenMDAO Group integrating the Ard wind plant as a sub-problem.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done and removed
| subprob_ard = om.SubmodelComp( | ||
| problem=ard_prob, | ||
| inputs=[ | ||
| "spacing_primary", | ||
| "spacing_secondary", | ||
| "angle_orientation", | ||
| "angle_skew", | ||
| "x_substations", | ||
| "y_substations", | ||
| ], | ||
| outputs=[ | ||
| ("aepFLORIS.power_farm", "electricity_out"), | ||
| ("tcc.tcc", "CapEx"), | ||
| ("opex.opex", "OpEx"), | ||
| "boundary_distances", | ||
| "turbine_spacing", | ||
| ], | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate the use of the submodel here! This is mostly for my own curiosity, but why did you choose this route instead of making a group of actual Ard components? Was it mostly to mimic the existing H2I converter style, or due to some of the problem formulation in Ard, or something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ard sets up a full model in openmdao, including driver etc. I took the submodel approach here to allow H2I to take full advantage of Ard (can do its own optimization, DOE, etc) and to not have to rewrite Ard (emphasis on the latter). This is particularly important because Ard is already bringing together many external codes that need special handling, so I thought it would be easier to have a clear line between the H2I and Ard systems.
| # add ard sub-problem | ||
| ard_input_dict = self.options["tech_config"]["model_inputs"]["performance_parameters"][ | ||
| "ard_system" | ||
| ] | ||
| ard_data_path = self.options["tech_config"]["model_inputs"]["performance_parameters"][ | ||
| "ard_data_path" | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll change this and push up directly, but I'm commenting here so you see this design paradigm. Instead of using:
ard_data_path = self.options["tech_config"]["model_inputs"]["performance_parameters"][
"ard_data_path"
]
you can just do
ard_data_path = self.config.ard_data_path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, thanks!
elenya-grant
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this Jared! I agree with Kaitlin's and John's feedback and added a few comments. My biggest request is to add a unit test or two for the ArdWindPlantModel into h2integrate/converters/wind/test/ to compliment the integration test you added with the example.
One other minor point is that, if possible and easy, it'd be nice if some of the inputs/outputs that exist in PYSAMWindPlantPerformanceModel were replicated in this wind plant model (I am not sure if this is possible given the subgroup setup you introduced).
I will also likely have questions in the future about connecting it with resource data, but that doesnt have to be addressed in this PR.
| # Post-process the results | ||
| model.post_process() | ||
|
|
||
| with subtests.test("Check wind generation"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Kaitlin - I think that having subtests on the value of the total wind generation profile and total solar generation profile would be a good so if other subtests fail (like LCOE or total electricity dispatched, etc), it will be easier to identify what causes other subtest failures.
| ] | ||
|
|
||
| resource_to_tech_connections: [ | ||
| # connect the wind resource to the wind technology |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick - can you change the comment in resource to tech connections to be about solar instead of wind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this driver config - I was curious about how this model could be used in a DOE or optimization given its different type of setup but happy to see this included!
| flag: True | ||
| file: "wind_h2_opt.sql" | ||
| includes: ["*"] | ||
| excludes: ["wind_resource.wind_resource_data"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could do:
excludes: ["*_resource_data"]to exclude wind and solar resource data from the output file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in the example it'd be nice if the optimization ran (set the optimization flag to True in the driver config file) but I know this optimization wouldn't be ideal for the test corresponding to this example. I'm making a note that after PR #313 is merged in, this would be good example to showcase loading the config files, modifying them, then initializing H2I with a dictionary rather than a filepath.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am also confused on why either of these CSV files are included because the turbine Cp curve information appears to be in examples/xx_wind_ard/ard_inputs/windIO-plant_turbine_NREL-5.0MW-126m-RWT.yaml
| ard_input_dict = self.config.ard_system | ||
| ard_data_path = self.config.ard_data_path | ||
|
|
||
| ard_prob = set_up_ard_model(input_dict=ard_input_dict, root_data_path=ard_data_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be out of scope for this PR - but in this Ard wrapper, could we update the site polygon boundaries input to Ard to be set as the site boundaries defined in the h2integrate plant config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't necessary in my opinion because the site boundaries for H2I are largely meaningless (for now). I think it's fine for Ard to have its own site boundary definition
| - ORBIT | ||
| - FinanceSE | ||
|
|
||
| ## Wind Resource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you specify what wind resource data is needed by ard and what the units should be? like that and needs wind speeds in m/s or whatever? Cause if folks are downloading resource data from the openmeteo web API - they may have wind speed in km/h (that's the default unit)
Ard integration for wind farm combined performance and cost model
This PR adds Ard as an available wind farm performance and cost model. Ard wraps many wind farm modeling capabilities including aerodynamics (FLORIS), cable array design (OptiWindNet), and cost/finance (WISDEM). Ard is built specifically for optimization, and as such includes common constraints, like arbitrary polygonal boundaries, the ability to use multiple discrete boundaries, and multiple parameterization approaches, like ordered grids and continuous location definitions.
The current (first iteration) wrapper only allows for ordered grid layouts.
Type of Contribution
General PR Checklist
CHANGELOG.mdhas been updated to describe the changes made in this PRdocs/files are up-to-date, or added when necessaryNew Technology Checklist
test_all_examples.pyexamples/README.mddocs/technology_models/docs/technology_models/technology_overview.mdsupported_models.pycreate_financial_modelinh2integrate_model.pydocs/developer_guide/coding_guidelines.mdRelated issues
This PR would be enhanced by allowing more resource type integrations, such as detailed in #237
Impacted areas of the software
CHANGELOG.md: note inclusion of Arddocs/_toc.yml: include Ard docs in TOCdocs/technology_models/technology_overview.md: include Ard in overviewdocs/technology_models/wind_plant_ard.md: high-level documentation/introduction to Ardexamples/xx_wind_ard/ard_inputs/NREL_Reference_5MW_126.csv: wind turbine definitionexamples/xx_wind_ard/ard_inputs/open-meteo-56.20N8.54E86m-short.yaml: abbreviated wind resource input file for Ardexamples/xx_wind_ard/ard_inputs/open-meteo-56.20N8.54E86m.yaml: wind resource input file for Ardexamples/xx_wind_ard/ard_inputs/power_thrust_table_ccblade_NREL-5p0-126-RWT.csv: wind turbine performance input for Ardexamples/xx_wind_ard/ard_inputs/windIO-plant_turbine_NREL-5.0MW-126m-RWT.yaml: wind turbine input definition for Ardexamples/xx_wind_ard/ard_inputs/windio.yaml: WindIO input file for Ardexamples/xx_wind_ard/h2i_inputs/driver_config.yaml: H2I driver input yaml for exampleexamples/xx_wind_ard/h2i_inputs/plant_config.yaml: H2I plant input yaml for exampleexamples/xx_wind_ard/h2i_inputs/tech_config.yaml: H2I tech input yaml for exampleexamples/xx_wind_ard/h2i_inputs/wind_pv_battery.yaml: top-level yaml input to run script for exampleexamples/xx_wind_ard/run_windard_pv_battery.py: main example run scripth2integrate/converters/wind/wind_plant_ard.pyWindArdCostComponent: mostly empty child ofCostModelBaseClassto allow H2I access tocost_yearparameterWindPlantArdModelConfig: defines inputs to Ard (ard_system, and ard_data_path)ArdWindPlantModel: OpenMDAO Group that includes and Ard OpenMDAO problem object as an OpenMDAOSubmodelComp.h2integrate/core/h2integrate_model.py: includewind_plant_ardincombined_performance_and_cost_modelsh2integrate/core/supported_models.py: includeArdWindPlantModelas "wind_plant_ard"pyproject.toml: include Ard as a dependencytests/h2integrate/test_all_examples.pytest_windard_pv_battery_dispatch_example: test Ard integration with H2Ipath/to/file.extensionmethod1: What and why something was changed in one sentence or less.Additional supporting information
The location used in the example is specific to IEA Task 50 WP 2. The current solar resource is not from the correct location.
Test results, if applicable