-
Notifications
You must be signed in to change notification settings - Fork 15
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
Adds twcrps for ensembles #644
Adds twcrps for ensembles #644
Conversation
4340ab3
to
3b051e7
Compare
I should also mention that I compared the results from |
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 @nicholasloveday . Some suggestions for docstrings
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.
-
Allen et al. (2023) (https://doi.org/10.1137/22M1532184) is paywalled. (Except for the supplementary materials, which are freely available). Is there also a non-paywalled reference available for these metrics? (I realise there isn't always a good, non-paywalled reference available).
-
I was looking at how Tail twCRPS for Ensembles and twCRPS for Ensembles appear in included.md when rendered (see: https://scores.readthedocs.io/en/228-documentation-testing-branch/included.html). They look good as is. However, I was wondering, should they perhaps be included as nested items underneath "Continuous Ranked Probability Score (CRPS) for Ensembles" - in a similar fashion to how the CRPS for CDFs functions appear in included.md? I really have no opinion on this - I imagine it is fine either way - I just wanted to raise the question.
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 did this a number of days ago and didn't realise it hadn't shown. Apologies.
|
Thank you for those helpful reviews. I have pushed up the changes. |
c4acf9f
to
11692bc
Compare
About
This pull request includes:
tail_twcrps_for_ensembles
for threshold weighting of the tailcrps_for_ensembles
tests (it can now take xr.datasets and broadcasting is tested)Tutorial
The tutorial is expected to be completed in a separate pull request.
Further extensions
In the future, I can to add other convenience functions for different kinds of threshold weighting (e.g., interval)
Checklist
Development for new xarray-based metrics
reduce_dims
,preserve_dims
, andweights
args.Testing of new xarray-based metrics
xr.Dataarrays
andxr.Datasets
Documentation