search

NREL / reV

Renewable Energy Potential (reV) Model
https://nrel.github.io/reV/
BSD 3-Clause "New" or "Revised" License
107 stars 24 forks source link

Gb/bespoke sc #361

Closed grantbuster closed 2 years ago

grantbuster commented 2 years ago

This needs to be tested full scale but it won't break anything so opening a PR now

codecov-commenter commented 2 years ago

Codecov Report

Merging #361 (5bfcdad) into main (5bfcdad) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 5bfcdad differs from pull request most recent head fa9d7ba. Consider uploading reports for the commit fa9d7ba to get more accurate results

@@           Coverage Diff           @@
##             main     #361   +/-   ##
=======================================
  Coverage   79.05%   79.05%           
=======================================
  Files         135      135           
  Lines       18256    18256           
=======================================
  Hits        14432    14432           
  Misses       3824     3824
Flag Coverage Δ
unittests 79.05% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5bfcdad...fa9d7ba. Read the comment docs.

ppinchuk commented 2 years ago

Full scale test failed... I think the bespoke meta is missing the sc_(row,col)_ind columns: https://github.com/NREL/reV/blob/a947b6ba8677224a939a1b2ac9b97d91525f06b2/reV/bespoke/bespoke.py#L355

Eagle test run directory: /shared-projects/rev/projects/weto/fy22/bespoke/rev/test_runs_conus_sc

Will add a review after patch

grantbuster commented 2 years ago

There's actually kind of a big issue that we need to make sure we're calculating LCOE(FCR) for the timeseries SAM simulation based on inputs from the optimized plant layout... Need to figure out with PJ how to link up the cost parameters...

ppinchuk commented 2 years ago

Yea, Owen, Travis, and I actually already talked about that. I was going to patch it in with a more general reV maintenance patch sometime this week. AFAIK, it should be straightforward by setting the OPEX and CAPEX correctly in the SAM config after the plant optimization

grantbuster commented 2 years ago

Yeah here's what I'm thinking: FCR and VOP would be taken from the input config and would be fixed, cap cost and fixed op cost would be based on the optimized plant. I'll just add optional inputs (required if lcoe_fcr is requested) to the sam_sys_inputs that would be capital_cost_per_mw and fixed_operating_cost_per_mw and then have the user input those.

ppinchuk commented 2 years ago

Couldn't you just compute capital_cost_per_mw and fixed_operating_cost_per_mw from the input config (before running the optimization)? capital_cost, fixed_operating_cost, and system_capacity are already required parameters, so the relationships capital_cost_per_mw = capital_cost / (system_capacity / 1000) and fixed_operating_cost_per_mw = fixed_operating_cost / (system_capacity / 1000) should hold prior to optimization

grantbuster commented 2 years ago

Yep we could do that too! I'm not sure if system_capacity is really required for bespoke but I agree that makes more sense than new keys

ppinchuk commented 2 years ago

Ah fair. We have been throwing it in anyways, probably because old habits die hard XD

grantbuster commented 2 years ago

Good idea though, i'll go ahead and implement that