-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: support for 'biomass' and 'other' generators, slight refactor #55
Conversation
|
@rouille see that PR here: Breakthrough-Energy/PowerSimData#71 |
@@ -9,11 +9,13 @@ | |||
} | |||
SCENARIO_RESOURCE_TYPES = ['wind', 'solar', 'ng', | |||
'coal', 'nuclear', 'geothermal', 'hydro'] | |||
ALL_RESOURCE_TYPES = SCENARIO_RESOURCE_TYPES + ['other inc. biomass'] | |||
ADDITIONAL_RESOURCE_TYPES = ['dfo', 'other inc. biomass', 'other', 'biomass'] |
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 for adding me onto the review. Since you're adding dfo
and biomass
to analyze_pg
then you should put them in SCENARIO_RESOURCE_TYPES
.
Now that I think about it, ADDITIONAL_RESOURCE_TYPES
is a relic from my original spot check code and I need to remove it.
Also we should eventually take type2label
and type2color
out of analyze_pg
so we don't have to have duplicates like this.
When will we be able to test these changes. I believe the new Eastern base now has other as generator type (no biomass though) and we could use these scenarios to test the changes. |
Scenario #307 is running, slowly. We'll probably have results to test by Monday morning. |
Tested successfully on Scenario #307. As a prerequisite, we need to have a version of powersimdata installed that contains the changes of both |
90cf533
to
06ed2b4
Compare
This change has been successfully tested, and the dependencies have been merged into PowerSimData (see Breakthrough-Energy/PowerSimData#71 and Breakthrough-Energy/PowerSimData#72) so I think this change is ready to merge. |
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.
Cool
06ed2b4
to
3311cc6
Compare
This adds support for analyzing 'biomass' and 'other' generators. There are a few places where I'm not sure if I did things the right way, particularly
constants.py
(@merrielle can you please look at these changes and make sure they don't break anything in plotting) andanalyze_set.py
(@rouille can you please look at these changes and make sure they don't break anything that we still use in this script).