-
Notifications
You must be signed in to change notification settings - Fork 40
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
Generalize generator types #656
Conversation
@FabianHofmann, @chrstphtrs, if you could help @danlivengood filling out the efficiency and emission values for PyPSA-Eur (currently set to None) that would be fantastic. |
What I'm seeing thus far is: CCGT, from the DEA data sheet tab 5 For the others: |
cef207b
to
0bae8af
Compare
Hi Dan, It looks like you need to differentiate your constants (emissions, efficiency etc.). Split Some general notes:
Editing your notes from above ( CCGT, from the DEA data sheet tab 5 For the others: |
7149e43
to
9e167b1
Compare
Hi Chris, |
|
Our |
300610c
to
5f7a61b
Compare
This is ready for review. I believe I got all hard coded generator types. Also, I have changed the default |
2c02d65
to
bbeeb40
Compare
b88e407
to
76e08a3
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.
Looks good to me. See my comments for some typo fixes in the docs.
onshore_wind_type = { | ||
v: k for k, v in grid.model_immutables.plants["type2label"].items() | ||
}["Onshore Wind"] |
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'm trying to understand the logic here, we do have "label2type" returned in get_plants
function which is populated into model_immutables.plants
, right? Also, we have "Onshore Wind" label only for Europe grid, no?
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.
You are right that we have label2type
and there is no need to revert the dict. For some reason, I believed I renamed the label for USA but it seems I did not. It should be:
def __init__(self):
self.type2label = {
"nuclear": "Nuclear",
"geothermal": "Geothermal",
"coal": "Coal",
"dfo": "Diesel Fuel Oil",
"hydro": "Hydro",
"ng": "Natural Gas",
"solar": "Solar",
"wind": "Onshore Wind",
"wind_offshore": "Offshore Wind",
"biomass": "Biomass",
"other": "Other",
"storage": "Storage",
}
self.curtailable2label = {
"solar": "Solar Curtailment",
"wind": "Onshore Wind Curtailment",
"wind_offshore": "Offshore Wind Curtailment",
}
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.
Done
pmin_by_type={"hydro": None}, | ||
pmin_by_id=None, | ||
curtailable={"solar", "wind"}, |
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.
Not sure whether we would like to keep mutable defaults 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.
I was trying to keep the original behavior of the function since we added hydro
to the list of curtailable resources.
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.
Yeah, I see where you come from. Just wonder whether we should move mutable defaults into if
logic in the function as we did it elsewhere, such as pmin_by_type = pmin_by_type if pmin_by_type else {"hydro": None}
.
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.
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.
Very nice. Thanks!
Pull Request doc
Purpose
Generalize generator type since these will depend on the selected grid model.
What the code is doing
Define constants for generators (such as color, label, emission, etc.) for each supported grid model.
Testing
Existing unit tests.
Where to look
powersimdata.network.constants.plants
module is where everything is definedUsage Example/Visuals
N/A
Time estimate
45min