-
Notifications
You must be signed in to change notification settings - Fork 17
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
LCOH Update and Custom Electrolyzer Costs #79
base: develop
Are you sure you want to change the base?
Conversation
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 really helpful, thank you!! Main things that are needed are (1) doc strings, (2) all costs need to be discounted in the same way and the same place (capex and opex cost functions), and (3) there are many unused functions that should be saved for a PR that uses them.
greenheart/simulation/technologies/hydrogen/electrolysis/PEM_costs_custom.py
Outdated
Show resolved
Hide resolved
import numpy as np | ||
|
||
|
||
def summarize_electrolysis_cost_and_performance(electrolyzer_physics_results, electrolyzer_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.
need doc strings with citation(s)
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've added doc strings!
greenheart/simulation/technologies/hydrogen/electrolysis/pem_cost_tools.py
Outdated
Show resolved
Hide resolved
greenheart/tools/eco/finance.py
Outdated
@@ -1237,6 +1242,19 @@ def run_profast_full_plant_model( | |||
output_dir = Path(output_dir).resolve() | |||
gen_inflation = greenheart_config["finance_parameters"]["profast_general_inflation"] | |||
|
|||
if "analysis_start_year" not in greenheart_config["finance_parameters"]: | |||
analysis_start_year = greenheart_config["project_parameters"]["atb_year"] + 2 | |||
if "installation_period_months" not in greenheart_config["finance_parameters"]: |
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 will fail if wind is not in the simulation. I think we should just raise errors on both of there with instructions to set the parameters.
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 moved analysis_start_year
to be added to config.greenheart_config["finance_parameters"]
in the __attrs_post_init__
method of GreenHeartSimulationConfig
.
I moved installation_period_months
to be set in setup_greenheart_simulation()
depending on whether wind_cost_results
has that as a variable or not.
return pf_config | ||
|
||
|
||
def create_and_populate_profast(pf_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 could be useful, but is not used. Let's hold off on this until we are ready to use it if it is not too hard.
return pf | ||
|
||
|
||
def run_profast(pf): |
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.
Again, not used so should probably be in a different PR
return sol, summary, price_breakdown | ||
|
||
|
||
def make_price_breakdown(price_breakdown, pf_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.
Could be useful now. Let's keep this even if we are not using it yet.
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.
Good progress, I noticed a few more things we really should fix. I'm sorry I did not notice them on my initial review.
) | ||
|
||
def calc_simple_refurb_schedule(self): | ||
annual_performance = self.electrolyzer_physics_results["H2_Results"][ |
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.
doc string
return refurb_simple | ||
|
||
def calc_complex_refurb_schedule(self): | ||
annual_performance = self.electrolyzer_physics_results["H2_Results"][ |
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.
doc string
return refurb_complex | ||
|
||
def make_lifetime_utilization(self): | ||
annual_performance = self.electrolyzer_physics_results["H2_Results"][ |
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.
doc string
|
||
|
||
def calc_electrolyzer_variable_om(electrolyzer_physics_results, greenheart_config): | ||
electrolyzer_config = greenheart_config["electrolyzer"] |
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.
doc string
|
||
@define | ||
class ElectrolyzerLCOHInputConfig: | ||
"""C |
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 the description did not get saved. Please include the description along with any relevant citations if necessary.
"""Make a custom price breakdown of primary cost items in $/unit of commodity. | ||
|
||
Args: | ||
price_breakdown (pd.DataFrame): price breakdown generated by ProFAST |
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.
The breakdown here does not match the breakdown that comes directly from profast. Can you provide more explanation on why and what this function is doing?
@@ -13,6 +13,7 @@ project_parameters: | |||
hybrid_electricity_estimated_cf: 0.492 #should equal 1 if grid_connection = True | |||
atb_year: 2025 | |||
cost_year: 2022 # to match ATB | |||
# installation_time: 0 #36 # months |
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.
As noted above, let's keep this input since we need it unless running ORBIT and I don't want a hidden hard-coded default somewhere in the code. Could set to "orbit" to use the value that is calculated by orbit
) | ||
|
||
|
||
TOL = 1e-3 |
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 tolerance is pretty loose, I would suggest going with <=1E-6 unless there is a good reason not to
@@ -441,4 +441,4 @@ def test_run_greenheart_optimize(subtests): | |||
lcoh_final = case.get_val("lcoh", units="USD/kg")[0] | |||
|
|||
with subtests.test("lcoh"): | |||
assert lcoh_final < lcoh_init | |||
assert lcoh_final <= lcoh_init |
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 testing the optimization function and should stay as <
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.
what is it trying to optimize? the greenheart config input file (tests/greenheart/input_files/plant/greenheart_config_onshore.yaml) is missing opt_options
.
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 is minimizing LCOH wrt the electrolyzer rating. The optimization stuff is added in the setup_greenheart
function called at the beginning of the test.
@@ -205,4 +205,4 @@ def test_run_greenheart_optimize_mpi(subtests): | |||
lcoh_final = case.get_val("lcoh", units="USD/kg")[0] | |||
|
|||
with subtests.test("lcoh"): | |||
assert lcoh_final < lcoh_init | |||
assert lcoh_final <= lcoh_init |
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 stay as <
if self.electrolyzer_config["complex_refurb"]: | ||
use_complex_refurb = True | ||
|
||
# complex schedule assumes stacks are replaced in the year they reach EOL |
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.
can we spell out end-of-life? make it easier for people not hip with the lingo
"Performance Schedules" | ||
] | ||
|
||
#: electrolyzer system capacity in kW |
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.
There's a mix of units in parentheses and some in text, maybe pick one for consistency?
* annual_performance["Annual Average Efficiency [kWh/kg]"].values | ||
) | ||
|
||
if "analysis_start_year" not in greenheart_config["finance_parameters"]: |
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.
Adding a note here, since I agree with @jaredthomas68. I think we should throw an error if these aren't set in the greenheart config
electrolyzer_eff_kWh_pr_kg: list[float] = field(init=False) | ||
electrolyzer_annual_h2_production_kg: list[float] = field(init=False) | ||
|
||
# simple_replacement_schedule: list[float] = field(init=False) |
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.
Remove commented out code?
import greenheart.tools.profast_tools as pf_tools | ||
|
||
|
||
# from greenheart.simulation.technologies.hydrogen.electrolysis.pem_cost_tools import ( |
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.
remove commented out import?
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.
To the extend that you'd be willing to add docstrings to this file that would be awesome since most of the methods are undocumented here. No need to add to the functions you don't modify but the ones you are adding/changing maybe you could document
@@ -658,7 +682,9 @@ def run_profast_lcoe( | |||
pf.set_params("maintenance", {"value": 0, "escalation": gen_inflation}) | |||
pf.set_params("analysis start year", greenheart_config["project_parameters"]["atb_year"] + 1) | |||
pf.set_params("operating life", greenheart_config["project_parameters"]["project_lifetime"]) | |||
pf.set_params("installation months", wind_cost_results.installation_time) | |||
pf.set_params( |
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.
Using the greeneheart config to set the "installation_time" removes the ability to set the installation time from ORBIT. Maybe in the run_wind_cost_model()
function we need to change it so if greenheart_config["project_parameters"]["installation_time"]
is not set that it uses the ORBIT output and logs the change? @jaredthomas68 thoughts?
A broader comment on the PR. @elenya-grant would you be able to add a note about why the test results are changing? Also, I think it's fine to leave the "unused profast tools", I believe some of them are used in the electrowinning work that is going to be brought in here in the next few weeks. |
Update to LCOH script and added custom electrolyzer cost functionality.
PR Checklist
CHANGELOG.md
has been updated to describe the changes made in this PRdocs/
files are up-to-date, or added when necessaryRelated issues
Impacted areas of the software
greenheart/tools/eco/finance.py
run_profast_full_plant_model()
: added electrolyzer variable O&M input, changed capacity input to be the electrolyzer rated capacity, changed long term utilization input to be dictionary of electrolyzer capacity factors per year.greenheart/tools/eco/electrolysis.py
run_electrolyzer_cost()
: updated for custom electrolyzer costgreenheart/simulation/technologies/hydrogen/electrolysis/PEM_costs_custom.py
: cost function to calculate electrolyzer capex and fixed o&m for "custom" electrolyzer cost modelgreenheart/simulation/technologies/hydrogen/electrolysis/pem_cost_tools.py
: summarizes electrolyzer physics results for appropriate use in LCOH scriptgreenheart/tools/profast_tools.py
: mostly unused for now, tools to automate using and post-processing ProFASTpath/to/file.extension
method1
: What and why something was changed in one sentence or less.Additional supporting information
Test results, if applicable