fix: power curves with missing cutoff

[joAschauer]

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

• convert_wind() function throws a warning if the power curve has no cutoff for high wind speeds i.e. if the last value is not zero.
• Cutout.wind() method now has a new argument add_cutoff. When set to True, this will modify the power curve by inserting a zero at the highest wind speed in the power curve.

Motivation and Context

see #314.

How Has This Been Tested?

Not really yet.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I tested my contribution locally and it seems to work fine.
• I locally ran pytest inside the repository and no unexpected problems came up.
• I have adjusted the docstrings in the code appropriately.
• I have documented the effects of my code changes in the documentation doc/.
• I have added newly introduced dependencies to environment.yaml file.
• I have added a note to release notes doc/release_notes.rst.
• I have used pre-commit run --all to lint/format/check my contribution

[euronion]

Thanks @joAschauer !

A few suggestions:

• make the allowed values for add_cutoff = True|False|"warn" with warn being the default (-> backwards compatibility)
• Move the warning from convert_wind(...) and implementation from wind(..) both into get_windturbineconfig(turbine, add_cutoff) right before the return statement at the end.
  

  atlite/atlite/resource.py

  Lines 35 to 75 in 549d0fd

  	def get_windturbineconfig(turbine):
  	"""
  	Load the wind 'turbine' configuration.
  	Parameters
  	----------
  	turbine : str or pathlib.Path
  	if str:
  	The name of a preshipped turbine from alite.resources.windturbine .
  	Alternatively, if a str starting with 'oedb:<name>' is passed the Open
  	Energy Database is searched for a turbine with the matching '<name>'
  	and if found that turbine configuration is used. See
  	`atlite.resource.get_oedb_windturbineconfig(...)`
  	if `pathlib.Path` is provided the configuration is read from this local
  	path instead
  	Returns
  	----------
  	config : dict
  	Config with details on the turbine
  	"""
  	assert isinstance(turbine, (str, Path))
  	if isinstance(turbine, str) and turbine.startswith("oedb:"):
  	return get_oedb_windturbineconfig(turbine[len("oedb:") :])
  	elif isinstance(turbine, str):
  	turbine_path = windturbines[turbine.replace(".yaml", "")]
  	elif isinstance(turbine, Path):
  	turbine_path = turbine
  	with open(turbine_path, "r") as f:
  	conf = yaml.safe_load(f)
  	return dict(
  	V=np.array(conf["V"]),
  	POW=np.array(conf["POW"]),
  	hub_height=conf["HUB_HEIGHT"],
  	P=np.max(conf["POW"]),
  	)

(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(...)

• Only print the warning if add_cutoff=warn

• We could change the behaviour in an upcoming version to add_cutoff = True by default. What do you think? In that case we'd need a warning that the implementation will change.

[joAschauer]

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]

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]

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]

Very nice!

A few final points:

• The correct term is "cut-out", not "cut-off", could you adjust all occurrences? (Sorry for using the incorrect term until now). It might also be better to use add_cutout_windspeed to avoid confusion with atlite.Cutout (your call!)
• FutureWarning: Announce change for v0.2.13 instead of "future version of atlite"
• Do you think we should add a check to _validate_turbine_config_dict(...) to ensure V is sorted ascending order?
• Can you add brief release notes into RELEASE_NOTES.rst?
• A test for the CI would be nice, this should suffice (add to test/test_resource.py:

def test_windturbineconfig_add_cutout():
    assert atlite.resource.get_windturbineconfig({'V': [ 0,  25], 'POW': [0.,1.], 'hub_height': 1., 'P': 1.}, add_cutout=True)["POW"][-1] == 0

[joAschauer]

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

[euronion]

Looking good! Thank you for the contribution!