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

Updated tutorial notebook for interval score #736

Merged

Conversation

reza-armuei
Copy link
Collaborator

@reza-armuei reza-armuei commented Nov 13, 2024

Please work through the following checklists. Delete anything that isn't relevant.

Development for new xarray-based metrics

  • Works with n-dimensional data and includes reduce_dims, preserve_dims, and weights args.
  • Typehints added
  • Add error handling
  • Imported into the API
  • Works with both xr.DataArrays and xr.Datasets if possible

Docstrings

  • Docstrings complete and follow Napoleon (google) style
  • Maths equation added
  • Reference to paper/webpage is in docstring. The preferred referencing style for journal articles is APA (7th edition)
  • Code example added

Testing of new xarray-based metrics

  • 100% unit test coverage
  • Test that metric is compatible with dask.
  • Test that metrics work with inputs that contain NaNs
  • Test that broadcasting with xarray works
  • Test both reduce and preserve dims arguments work
  • Test that errors are raised as expected
  • Test that it works with both xr.DataArrays and xr.Datasets

Tutorial notebook

  • Short introduction to why you would use that metric and what it tells you
  • A link to a reference
  • A "things to try next" section at the end
  • Add notebook to Tutorial_Gallery.ipynb
  • Optional - a detailed discussion of how the metric works at the end of the notebook

Documentation

Copy link
Collaborator

@Steph-Chong Steph-Chong left a comment

Choose a reason for hiding this comment

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

Thanks @reza-armuei. Overall it looks good to me. I have made a few comments about some very minor things.

tutorials/Quantile_Interval_And_Interval_Scores.ipynb Outdated Show resolved Hide resolved
tutorials/Quantile_Interval_And_Interval_Scores.ipynb Outdated Show resolved Hide resolved
"\n",
"where $S$ is the scoring function (here *quantile interval score*), $q_l$ and $q_u$ are lower and upper quantile forecasts, and $y$ is observation, $\\alpha_l$ and $\\alpha_u$ are lower and upper quantile level. Lower values of the quantile interval scores are better. As you can see, this score penalizes the width of interval as well as the extent to which observation falls outside of the interval forecast (i.e., under-prediction penalty when observation is larger than upper-quantile forecast, and over-prediction penalty when observation is smaller than lower-quantile forecast).\n",
"where:\n",
"- $S$ is the scoring function (here quantile interval score),\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

The dotpoints are not rendering correctly in readthedocs. The screenshot below shows how they are currently rendering in readthedocs:
image

This can usually be fixed by adding an empty line before the first dot point.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tennlee fixed this post-merge (by adding an empty line before the first dot point).

tennlee and others added 2 commits November 13, 2024 21:21
Co-authored-by: Stephanie Chong <[email protected]>
Signed-off-by: Tennessee Leeuwenburg <[email protected]>
Co-authored-by: Stephanie Chong <[email protected]>
Signed-off-by: Tennessee Leeuwenburg <[email protected]>
Co-authored-by: Stephanie Chong <[email protected]>
Signed-off-by: Tennessee Leeuwenburg <[email protected]>
tennlee and others added 2 commits November 13, 2024 21:42
Co-authored-by: Stephanie Chong <[email protected]>
Signed-off-by: Tennessee Leeuwenburg <[email protected]>
Co-authored-by: Stephanie Chong <[email protected]>
Signed-off-by: Tennessee Leeuwenburg <[email protected]>
@tennlee
Copy link
Collaborator

tennlee commented Nov 13, 2024

The remaining issues relate to notebook rendering. It is easier for me to review and fix these post-merge, which is what I shall do. Thanks both.

@tennlee tennlee merged commit 9ce7bc8 into nci:develop Nov 13, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants