Skip to content
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

Add wind power potential and wind profile indicators #1471

Merged
merged 15 commits into from
Sep 28, 2023
Merged

Add wind power potential and wind profile indicators #1471

merged 15 commits into from
Sep 28, 2023

Conversation

huard
Copy link
Collaborator

@huard huard commented Sep 8, 2023

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGES.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • Adds two indices and their indicators: wind_profile and wind_power_potential.

Does this PR introduce a breaking change?

No

Other information:

Ideally we`d add an indicator to compute air density, which is an optional input to the wind power potential.

@github-actions github-actions bot added docs Improvements to documenation indicators Climate indices and indicators labels Sep 8, 2023
Copy link

@laura-vanvliet laura-vanvliet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @huard, I think overall this looks great! There is one important change due to another typo in the Tobin (2018) paper, and some suggestions for the function descriptors.

xclim/indices/_conversion.py Outdated Show resolved Hide resolved
xclim/indices/_conversion.py Outdated Show resolved Hide resolved
xclim/indices/_conversion.py Outdated Show resolved Hide resolved
For non-standard air density (:math:`\rho`), the wind speed is scaled using
:math:`v_n = v \left( \frac{\rho}{\rho_0} \right)^{1/3}`.

Note that the temporal resolution of the wind speed time series has a significant influence on the results,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this is not necessary, but you could also note that temporal resolution may affect aggregation in time

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to

Note that the temporal resolution of the wind speed time series has a significant influence on the results,
even when aggregated at longer time scales. Total annual power production will differ substantially if estimated
from hourly or daily wind speeds, due to the non-linearity of the production factor.

Is this what you meant ?

xclim/indices/_conversion.py Outdated Show resolved Hide resolved
tests/test_indices.py Outdated Show resolved Hide resolved
@huard huard requested a review from aulemahal September 18, 2023 17:38
Copy link
Collaborator

@aulemahal aulemahal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

May be you could add real tests in test_atmos ?

xclim/indices/_conversion.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the approved Approved for additional tests label Sep 26, 2023
@laura-vanvliet
Copy link

laura-vanvliet commented Sep 27, 2023 via email

@huard
Copy link
Collaborator Author

huard commented Sep 28, 2023

Ok, changed the wording of the docstring to simplify, and added a note explaining how to aggregate instantaneous power potential at lower frequencies. Tests have also been improved in the process, including one demoing the conversion to daily power production in MWh from 3-hourly values.

Also fixed a bug in to_agg_units, but I note there are no explicit tests for this function.

@huard huard merged commit 9394fbc into master Sep 28, 2023
12 checks passed
@huard huard deleted the fix-1458 branch September 28, 2023 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests docs Improvements to documenation indicators Climate indices and indicators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Indicator for wind power
3 participants