-
Notifications
You must be signed in to change notification settings - Fork 48
Add electric storage size class #564
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
Only adds size class in inputs processing, other code modifications pending.
Bill-Becker
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.
Great progress. I left some comments and requests. Can you also check the results and proforma calcs to see if we need to update anything there? Please also add a test to validate that the size_class -> cost params are being properly assigned. Let's also work toward updating battery costs to the new ATB values (zero for constant, 4% O&M, scale 2025 values from 2023 dollars to 2025 dollars using CPI - I can show you what we did last year).
| end | ||
|
|
||
| """ | ||
| get_pv_cost_params(; installed_cost_per_kw, size_class, tech_sizes_for_cost_curve, |
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.
update docstring for storage instead of pv
| min_kw::Real = 0.0, | ||
| max_kw::Real = 1.0e9, | ||
| electric_load_annual_peak::Real = 0.0, | ||
| electric_load_average_peak::Real = 0.0 |
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 "_peak" assuming this is just the average load
| { | ||
| "size_class": 1, | ||
| "name": "Residential", | ||
| "kw_tech_sizes_for_cost_curve": [0, 40], |
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.
let's come up with a better name since we don't consider storage a "tech", and we're not doing a cost curve with this array for storage. How about "size_class_bounds"?
| defaults[matching_default] | ||
| end | ||
|
|
||
| # STEP 2: Handle installed costs |
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.
We should be able to remove/avoid all this validation and separate handling for scalar versus array cost parameters, since we're not allowing an array for storage.
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.
Yup, I corrected those instances already.
|
|
||
| @info s.installed_cost_per_kw, s.size_class, s.electric_load_annual_peak, s.electric_load_average_peak | ||
|
|
||
| installed_cost_per_kw, installed_cost_per_kwh, installed_cost_constant, size_class, |
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 O&M cost % is included in the defaults .json, implying that it changes by size_class, but we're not assigning it by size_class here. I'm fine to take it out of that file and just used a fixed default of 2.5% (will become 4% from the new ATB). ATB uses the same % for all three of their size_classes
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.
Makes sense, since the degradation wear and tear will vary by cycling and not size class.
src/core/scenario.jl
Outdated
| storage_dict["off_grid_flag"] = settings.off_grid_flag | ||
|
|
||
| electric_load_annual_peak = maximum(electric_load.loads_kw) | ||
| electric_load_average_peak = sum(electric_load.loads_kw) / Int(8760*settings.time_steps_per_hour) |
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.
Why do we need to make sure it's converting to Int here?
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.
No reason for it, updated.
bug fix for existing tests
No description provided.