-
Notifications
You must be signed in to change notification settings - Fork 245
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
allow usage of own powerplants for selected country #69
Conversation
Hey! I think the setup it nice and should work well. Just as a counterproposal: There is one optional, additional dataset, called custom_powerplants.csv in the resource folder. This is added to the standard powerplant list if in config.py the default custom_powerplants: false is either set to custom_powerplants: Country in ['DE', 'FR'] & YearCommisioned < 2050 The same filter option could be enabled for the standard powerplant list, e.g. powerplants_filter: Country not in ['DE', 'FR'] & YearCommisioned < 2050 (The powerplants_filter should come first in the config.yaml, though. ) This could strongly reduce the code such as : def add_custom_carriers(ppl):
custom_ppl_query = snakemake.config['electricity']['custom_powerplants'']
if not custom_ppl_query:
return ppl
# import custom_ppl here
if isinstance(custom_ppl_query, str):
custom_ppl.query(custom_ppl_query, inplace=True)
return ppl.append(cusom_ppl) And of course the filter for the standard ppl, not as a function but in the 'main' code. |
Couple of comments:
|
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.
A couple of general comments about your code. Don't implement them. Instead, go ahead with @FabianHofmann's proposal.
scripts/build_powerplants.py
Outdated
@@ -18,6 +18,31 @@ def country_alpha_2(name): | |||
cntry = pyc.countries.get(official_name=name) | |||
return cntry.alpha_2 | |||
|
|||
def add_my_carriers(ppl): | |||
switch = snakemake.config['electricity']['my_carriers_switch'] | |||
if switch == 'replace-all' or switch == 'replace-selection' or switch == 'add': |
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.
in such case use a
if switch in ('replace-all', 'replace-selection', 'add'):
form
scripts/build_powerplants.py
Outdated
if switch == 'replace-all' or switch == 'replace-selection': | ||
to_drop = ppl[ppl.Country == countries_dict[country]] | ||
if switch == 'replace-selection': | ||
to_drop = to_drop[to_drop.Fueltype.isin(add_ppls.groupby('Fueltype').mean().index)] |
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.
add_ppls.groupby('Fueltype').mean().index
is a very strange construct.
Do you mean?
add_ppls.Fueltype.unique()
scripts/build_powerplants.py
Outdated
search_pattern = [str(int(year)+x) for x in range(1,2050-int(year))] | ||
logger.info('restricting build year of generators to ' + str(year) + '...') | ||
for pattern in search_pattern: #do it in forloop+contains instead of map as YearCommissioned might have the weirdest formats | ||
ppl = ppl[ppl['YearCommissioned'].str.contains(pattern) == False] |
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.
x == True
or x == False
is a bit silly, when you think about what it means. Use x
for the former and not x
or ~ x
(in this case for pandas), instead.
More importantly,
for pattern in search_pattern:
ppl = ppl[~ppl['YearCommissioned'].str.contains(pattern)]
has very bad performance characteristics, as you create copies of the power plant list in a loop. Take advantage of the fact that ~ ppl['YearCommissioned'].str.contains(pattern)
creates a slim boolean selection mask, ie. instead start with take all, and then narrow the mask down:
take_b = pd.Series(True, index=ppl.index)
for pattern in search_pattern:
take_b &= ~ppl['YearCommissioned'].str.contains(pattern)
ppl = ppl.loc[take_b]
scripts/build_powerplants.py
Outdated
|
||
def restrict_buildyear(ppl): | ||
year = snakemake.config['electricity']['restrict_buildyear'] | ||
search_pattern = [str(int(year)+x) for x in range(1,2050-int(year))] |
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.
Took me a while to reason about, what you are searching for, instead use:
search_pattern = [str(x) for x in range(int(year),2050)]
or even
search_pattern = map(str, range(int(year), 2050))
Hi both, thanks for your input. I made few changes from both your proposals, yet I would like to keep the separation - filtering the year is a different issue than replacing or adding custom powerplants. |
I did a histogram to see how many of the data is wrong processed by just casting to int. It's less than 1%, so I won't do any preprocessing as there are already so many NaN values, this will not have such a huge impact |
close this in favour of #84 |
Merged in feature/ref-config (pull request PyPSA#69) Disable more configurations in reference scenario to be more realistic * Disable more configurations in reference scenario to be more realistic
in build_powerplants.py add the feature to add own carriers (add_my_carriers) and restrict to a specific build year (restrict_buildyear), which both can be configured in config.yaml (my_carriers_for_countries, my_carriers_switch, restrict_buildyear).