feat: Implement Advanced Spatial (FSS) and Categorical Verification Metrics by GiGiKoneti · Pull Request #7 · mllam/mllam-verification

GiGiKoneti · 2026-06-16T22:21:10Z

Describe your changes

This PR implements advanced spatial and categorical verification metrics as proposed in Issue #6:

fractions_skill_score() (FSS) in statistics.py for spatial scale-selective verification.
equitable_threat_score() (ETS) in statistics.py for categorical skill evaluation.
brier_score() in statistics.py for probabilistic threshold verification.
plot_fss_scale() in plot.py for scale-selective FSS visualization.
Complete test suites for FSS, ETS, Brier Score, and FSS plotting.
Added changes to CHANGELOG.md.

No new dependencies are required.

Issue Link

Closes #6

Type of change

✨ New feature (non-breaking change that adds functionality)

Checklist before requesting a review

My branch is up-to-date with the target branch - if not update your fork with the changes from the target branch (use pull with --rebase option if possible).
I have performed a self-review of my code
For any new/modified functions/classes I have added docstrings that clearly describe its purpose, expected inputs and returned values
I have placed in-line comments to clarify the intent of any hard-to-understand passages of my code
I have updated the documentation to cover introduced code changes
I have added tests that prove my fix is effective or that my feature works
I have given the PR a name that clearly describes the change, written in imperative form.
I have requested a reviewer and an assignee

…semble evaluation

…etrics

GiGiKoneti · 2026-06-24T19:35:14Z

@mfroelund
Please review this PR

mfroelund

Looks really good, thanks for all your work. I've put a bunch of comments to consider, but overall very nice improvements!

mfroelund · 2026-06-25T08:17:48Z

+        **stats_op_kwargs: Additional keyword arguments.
+
+    Returns:
+        Dataset or DataArray with ETS values. Range: -1/3 to 1.


Just to be sure, should it be 0 - 1 and not -1/3 - 1, or what does -1/3 signify ?

The range of the Equitable Threat Score (ETS) is indeed [-1/3, 1].

A score of 1 indicates a perfect forecast.

A score of 0 indicates no skill (performing no better than a random forecast).

A score of -1/3 represents the worst possible forecast under certain conditions (specifically, when predictions and observations are completely mismatched with a base rate of 0.5, as discussed in Section 3a of Hogan et al., 2010).

I've kept -1/3 to 1 as the range in the docstring and added the citation to the Hogan et al. (2010) paper to clarify this.

mfroelund · 2026-06-25T08:43:16Z

+    da_prediction: xr.DataArray,
+    threshold: float,
+    window_sizes: list[int],
+    spatial_dims: Optional[list[str]] = None,


Would it make sense to put ["x", "y"] as a default value, instead of doing that inside the function on line 643? That way, it's more clear from the function argument list, what spatial_dims are used if none are provided?

If accepted, then the Optional part of the type hint should be omitted as described in below comment.

Good suggestion. I've updated the function signature to specify spatial_dims: list[str] = ["x", "y"] and removed the redundant fallback check inside the function body.

GiGiKoneti · 2026-06-25T11:14:16Z

@mfroelund
I have updated the PR to address all your comments:

FSS Plotting (plot_fss_scale): Changed default parameter values for spatial_dims to ["x", "y"] and hue to "datasource", removing the conditional checks. Also added logging via loguru when the requested hue is not found in coordinates/dimensions.
Statistics:
- Updated the DOI reference link for Fortin et al. (2014) in spread_skill_ratio.
- Added a concrete example for "yes" events in the Equitable Threat Score docstring.
- Refined the comment explaining the mse_ref == 0 edge-case in fractions_skill_score.
Documentation: Added the visualization row entries for Rank histogram and FSS vs scale to the README.md table.
CI & Repository: Cleaned up accidental environment files (.venv-test/, .precommit-home/, and uv.lock) and updated the CI pre-commit workflow to run hooks directly to avoid cache service outages.

All tests and pre-commit hooks are passing locally. Please let me know if you need anything else!

mfroelund

Final small comments. Also there are two comments from the previous review, that I still would like to hear your thoughts on.

mfroelund · 2026-06-25T11:31:39Z

+        ...     spatial_dims=["x", "y"],
+        ... )
+    """
+    if spatial_dims is None:


I suggest to do the same here as in 48498e9#diff-15c7e6b996e82bcc0f02abdd0aa702dcc8d0afae552cec7872cbc1005081c896R617

I have updated the fractions_skill_score() function signature to default directly to spatial_dims: list[str] = ["x", "y"] and removed the internal check, matching the design pattern of plot_fss_scale().

mfroelund · 2026-06-25T11:35:22Z

I don't see a difference between Model A and Model B. Could maybe be good to add some noise to make the example look more realistic.

I've regenerated the plot with realistic synthetic data: Model A represents a high-skill forecast with low noise, while Model B has higher noise and a spatial displacement offset to simulate the double-penalty behavior. The new plot clearly illustrates Model A's superior performance across scales.

GiGiKoneti · 2026-06-25T12:36:48Z

@mfroelund
Thanks for your detailed review..
Is there anything else u find which can be improved ?

GiGiKoneti force-pushed the feat/fss-categorical-metrics branch from 52fa97c to 91b986d Compare June 16, 2026 22:23

GiGiKoneti added 2 commits June 17, 2026 03:55

feat(metrics): add CRPS, spread-skill ratio and rank histogram for en…

2b29451

…semble evaluation

feat: implement advanced spatial (FSS) and categorical verification m…

d57e3cd

…etrics

GiGiKoneti force-pushed the feat/fss-categorical-metrics branch from 91b986d to d57e3cd Compare June 16, 2026 22:26

ci: run pre-commit directly to bypass cache service outages

fa5adb8

mfroelund reviewed Jun 25, 2026

View reviewed changes

GiGiKoneti force-pushed the feat/fss-categorical-metrics branch from 5e3b132 to 48498e9 Compare June 25, 2026 10:44

mfroelund reviewed Jun 25, 2026

View reviewed changes

GiGiKoneti force-pushed the feat/fss-categorical-metrics branch 2 times, most recently from eb74912 to 2837ad7 Compare June 25, 2026 12:14

style: address PR review comments and update documentation table

d12a745

GiGiKoneti force-pushed the feat/fss-categorical-metrics branch from 2837ad7 to d12a745 Compare June 25, 2026 12:18

Uh oh!

Conversation

GiGiKoneti commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Issue Link

Type of change

Checklist before requesting a review

Uh oh!

GiGiKoneti commented Jun 24, 2026

Uh oh!

mfroelund left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GiGiKoneti Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GiGiKoneti commented Jun 25, 2026

Uh oh!

mfroelund left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GiGiKoneti commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

GiGiKoneti commented Jun 16, 2026 •

edited

Loading

GiGiKoneti Jun 25, 2026 •

edited

Loading