-
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
Refactor hopp solver #305
Refactor hopp solver #305
Conversation
…ive technology dispatches
…ctive technology dispatches
…ctive technology dispatches
223fc0c
to
ae0fcb6
Compare
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 it looks really great in general. I do think that while we go about refactoring different pieces of the solver that it would be valuable to add docstrings.
At a minimum I'd like to see a docstring for each individual technology dispatch and for each objective function.
I think it would be valuable to add docstrings for the functions you've added/changed even internal functions to help make it easier for future developers to add additional technologies and start to understand the various pyomo connections.
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 agree with Kaitlin. This looks fine, but some doc strings would go a long way. Since you are up against a milestone I'm fine with merging it in now, but at least make an issue to add the doc strings. I also pointed out one typo that should be fixed prior to merge.
hopp/simulation/technologies/dispatch/power_sources/wave_dispatch.py
Outdated
Show resolved
Hide resolved
0103e50
to
b63c22c
Compare
Refactor HOPP solver to be more modular
This PR generalizes the HOPP dispatch structure to more easily allow for the inclusion of new technologies and facilitate debugging of the dispatch codes. This will also support adding new dispatch strategies.
Related issue
#307
#308
Impacted areas of the software
The dispatch classes and code.
Additional supporting information
Most of the changes are moving code from
hybrid_dispatch.py
into the respective technology dispatch classes.Other significant changes are calling general functions from
HybridDispatch
such asself.power_sources[tech]._dispatch._create_variables()
while looping over technologies instead of building the attribute to be retrieved with through strings. This will make debugging easier.The remaining changes are mainly formatting the code for improved readability.
Test results, if applicable
The tests will be passing.