-
Notifications
You must be signed in to change notification settings - Fork 114
Refactor battery enums #602
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
Conversation
…pv smoothing to ensure manual and custom lined up between dispatch versions
…ot at least one year long
…tomated economic dispatch since the numbers would likely change
| } | ||
| } | ||
|
|
||
| // Re-compute PV AC forecast for AC connected batteries |
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.
@sjanzou This code change is the reason the number of violations increased in the pv smoothing test. The new AC look ahead number includes additional losses and therefore I'd argue it's more accurate. Let me know if it makes sense to update the numbers in the test or solve the test failure some other way.
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 probably re-evaluate with the code from EPRI - although it will not include the additional losses - maybe if we use the updated pv profile as input to the Python from EPRI - we can match the violation count to the new SAM count...
sjanzou
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.
looks good!
dguittet
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.
Minor requests from me. After those, I think it's good to go!
ssc/cmod_battery.cpp
Outdated
| look_ahead = true; | ||
| look_ahead = batt_weather_forecast == dispatch_t::WEATHER_FORECAST_CHOICE::WF_LOOK_AHEAD; | ||
| look_behind = batt_weather_forecast == dispatch_t::WEATHER_FORECAST_CHOICE::WF_LOOK_BEHIND; | ||
| input_forecast = batt_weather_forecast == dispatch_t::WEATHER_FORECAST_CHOICE::WF_CUSTOM; |
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 the standard semantics for this is the case keyword. That'll also make sure batt_weather_forecast is within the correct bounds for `dispatch_t::WEATHER_FORECAST_CHOICE1
ssc/cmod_battery.cpp
Outdated
| if (batt_dispatch == dispatch_t::FOM_AUTOMATED_ECONOMIC || batt_dispatch == dispatch_t::FOM_PV_SMOOTHING) { | ||
| look_ahead = batt_weather_forecast == dispatch_t::WEATHER_FORECAST_CHOICE::WF_LOOK_AHEAD; | ||
| look_behind = batt_weather_forecast == dispatch_t::WEATHER_FORECAST_CHOICE::WF_LOOK_BEHIND; | ||
| input_forecast = batt_weather_forecast == dispatch_t::WEATHER_FORECAST_CHOICE::WF_CUSTOM; |
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.
same, look_ahead, etc should all be False to start then one should be toggled on
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.
These are flagged to false in the header file. I'll proceed with the case statement for clarity.
|
|
||
| int batt_forecast_choice = as_integer("batt_dispatch_wf_forecast_choice"); | ||
| if (is_assigned("batt_pv_clipping_forecast")) { | ||
| p_pv_clipping_forecast = as_vector_ssc_number_t("batt_pv_clipping_forecast"); |
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 remember you asking about lifetime forecasts, but herebatt_pv_clipping_forecast is 1 year only? Please specify in var table if its 1 yr forecast, if it isn't already
…pdate forecast variables to accept lifetime lengths as available. Update vartable descriptions to indicate length requirements. Remove errorneous kw to w conversion when forecast is labelled in kw
Reduce the number of dispatch algorithms to places where the underlying algorithm is different, and add new enums for weather (generation) forecast and load forecasting. These are fully fleshed out for FOM, BTM will come in another pull request. Pairs with SAM PR: NREL/SAM#681
New forecast enum numbers:
FOM:
BTM: