PyPSA / atlite

Atlite: A Lightweight Python Package for Calculating Renewable Power Potentials and Time Series
https://atlite.readthedocs.io
254 stars 86 forks source link

fix: power curves with missing cutoff #316

Closed joAschauer closed 9 months ago

joAschauer commented 11 months ago

Closes #314

Change proposed in this Pull Request

First proposal to fix #314. It is still to discuss if the appended cutoff wind speed should be slightly higher than the last entry in the power curve. Currently, np.interp will lead to 0 if the exact cutoff wind speed is met.

Description

Motivation and Context

see #314.

How Has This Been Tested?

Not really yet.

Type of change

Checklist

euronion commented 10 months ago

Thanks @joAschauer !

A few suggestions:

(Since that method is called internally by .wind(...) it will yield the same result, but people can also pre-load the turbine and modify the power curve manually before passing it to .wind(...)

joAschauer commented 10 months ago

Hi @euronion,

thanks for the feedback, I can get back to this soon.

I put the warning and modification code in convert_wind() and .wind() respectively, since the user can also pass a dict as turbine argument to .wind(). In this case, a missing cutoff will be overlooked if the code is in get_windturbineconfig(). If this is acceptable to you, I can change it as you suggested above.

I can also try to add a deprecation warning, I think it's reasonable to default to add_cutoff=True in future versions.

euronion commented 10 months ago

Good thinking with the custom power curves!

I generally lean towards making everyone responsible for the correctness of their own input (custom power curves), but I think keeping the warning is also fair.

How about putting the warning + the modification into a single function and adding that function to get_windturbineconfig() (warning + modifying) and .wind() (warning only) then? Then we have both cases covered with the same functionality.

joAschauer commented 10 months ago

Hi @euronion, I made an update and moved the turbine config validation to a new function. I did not implement the "warn" option for add_cutoff as I think it is clearer to leave the two options True/False. Keeping the default at False for now will not introduce any backwards incompatible changes.

I only call the validation function in get_windturbineconfig() cause otherwise we would end up with two concurrently thrown equal warnings.

euronion commented 10 months ago

Very nice!

A few final points:

joAschauer commented 10 months ago

hi @euronion I addressed your points. You can check again and merge if you are happy with it.