New score - NSE

[nikeethr]

$\color{green}\Large\text{READY TO MERGE}$

note: some minor adjustments and finishing touches are still being worked on, but the core functionality is ready for review

Description

Revisit implementation of NSE in #217 which is now stale. Details in issue: #814

Review

• Design discussion (@tennlee and @nikeethr)
• Address first round comments (@nikeethr)
• Software review (@tennlee)
• Documentation review (@Steph-Chong) (NOTE: first round changes have been addressed, ticking it for now until further feedback)
• Science review (@durgals)

Science Review

Apart from the general guidelines, specifically it would be good to validate the following:

• Handwritten test: see tests/continuous/test_nse.py:TestNseScore (note: just the math in the docstring)
• Tutorial: src/tutorials/NSE.ipynb
• NSE scoring logic: see src/scores/continuous/nse_impl.py:NseScore
• Public API docstrings: see src/scores/continuous/standard_impl.py:nse

Software changes

(NOTE: any major structural changes have now been offloaded to #830 as such I'm removing some information from this PR description - so that they don't cause confusion...)

• Added check_weights function: parially-addresses Add a UserWarning (or maybe add an exception) for negative weights #829
• Added nse score in scores.continuous.nse_impl.py:nse
• nse_impl also contains some checks that are contained in a NseUtils namespace. It is likely that these can be abstracted to apply to all Hydro metrics (in the first instance) and for all scores (eventually). (NOTE: out of scope for this PR - will be covered in put hydro scores in hydro_impl or similar #837)
• The key difference with nse_impl and most other scores is that the inner logic ONLY works with xr.Dataset as an intentionally imposed constraint. This simplifies any checking and compatibility of internal logic.
  • This DOES NOT mean nse_impl is incompatible with dataarrays, INSTEAD...
  • There is external logic that then converts between datasets and dataarrays and stores some metadata to convert between the two.
  • Note that this is a fairly abstractable pattern - but in light of keeping this specific to NSE the implementation is more "hardwired" and any generic structural implementation that can apply to any score is offloaded to Improve XarrayLike types to consolidate xr.DataArray and xr.Dataset #830

Misc:

• support for pixi environments in pyproject.toml moved to Add pixi to pyproject.toml #834

---

Checklist

Development (done)

• initial commit & placeholder
• refine this list to incorporate checklist below
• Imported into the API
• Implement score (note: untested)
• Works with n-dimensional data and includes reduce_dims, preserve_dims, and weights args.
• Address divide-by-zero & negative weighting issues
• Refactor due to code bloat
• develop user warning strategy for NSE
• Typehints added
• Add error handling

---

Public Docstring (done)

• Docstrings complete and follow Napoleon (google) style
• Maths equation added
• Reference to paper/webpage is in docstring.
• Code example added

Private Docstring (done)

• Internal API docs & comments - main for developers

---

Unit Test (done)

• 100% unit test coverage (main priority)
• Test that metrics work with inputs that contain NaNs
• 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
• Test that metrics work with inputs that contain NaNs
• Test that metric is compatible with dask. (handled as part of mse, but still worth checking if we get a delayed output.)
• finish up handwritten test
• fix compatibility issues with .add_note - doesn't exist in 3.10.

---

Tutorial (mostly done)

• Use pre-existing notebook in (stale) add NSE function #217, revising it to work with the API in this PR
• other things to try - section

---

API Documentation

• Add the score to the API documentation
• Add the score to the [included list of metrics and tools (https://github.com/nci/scores/blob/develop/docs/included.md)
• Double check rendering, docstring, reference format, typos, grammar etc.

---

[tennlee]

@durgals @nikeethr this looks good to me and I have completed my final code review. @durgals can you confirm if your comments have been fully addressed? If so, I will proceed to merge this PR.

[durgals]

@durgals @nikeethr this looks good to me and I have completed my final code review. @durgals can you confirm if your comments have been fully addressed? If so, I will proceed to merge this PR.

Hi @tennlee and @nikeethr . All my comments have been addressed, so this PR is good to go.