-
Notifications
You must be signed in to change notification settings - Fork 46
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
make battery heuristic general so other generation tech besides wind and pv can use the heuristic #377
make battery heuristic general so other generation tech besides wind and pv can use the heuristic #377
Conversation
…and pv can be used
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 to merge to me!
…o feature/generalize-dispatch
@@ -674,12 +674,12 @@ def simulate_with_dispatch( | |||
|
|||
def battery_heuristic(self): | |||
tot_gen = [0.0] * self.options.n_look_ahead_periods |
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 this be simplified to the following?
tot_gen = np.zeros(self.options.n_look_ahead_periods)
for power_source in self.power_sources.keys():
if "battery" in power_source or "grid" in power_source:
continue
tot_gen += self.power_sources[power_source].dispatch.available_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.
Yes and done
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 this is missing from above the for loop: tot_gen = np.zeros(self.options.n_look_ahead_periods)
. If you iteratively add a list, it'll just be a really long list.
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 catch. I'm not sure numpy will work without having knock-on effects, but I'll try it
tests/hopp/test_dispatch.py
Outdated
wave_resource_file = ROOT_DIR / "simulation" / "resource_files" / "wave" / "Wave_resource_timeseries.csv" | ||
|
||
desired_schedule=desired_schedule = 8760*[20] |
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 like there is an extra desired_schedule=
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.
Thanks, fixed
…and pv can be used
Generalize heuristic load following to any gen tech
This PR allows any technology to be dispatched with the load following heuristic, not just wind and pv.
Related issue
Impacted areas of the software
hopp/simulation/technologies/dispatch/hybrid_dispatch_builder_solver.py
Additional supporting information
This came up while working on the profast financial model and needs to get merged in prior to the financial model work
Test results, if applicable
Passing