Add string option for metrics

[stkistner]

An idea for 'special metrics':

The concept is that metrics can also produce string outputs. The main purpose for this is to add auxiliary info to the metrics, e.g. for Peak Ratios we often would like to know how many peaks there were:

While not currently used in modelskill itself, but applications using modelskill.

The downside is once the value is set, rounding of numbers would be tricky... But I don't know of any alternative (quick) solution to provide auxiliary metrics without breaking everything.

[ecomodeller]

Do you have other examples than the peak ratio, where it would be relevant with additional output?

[stkistner]

Do you have other examples than the peak ratio, where it would be relevant with additional output?

Our "old" matlab repo included percentages relative to the observations mean, and a statement that SI is unbiased, e.g. here is a full example, taken from the Matlab equivalent:

[daniel-caichac-DHI]

I agree that the (unbiased) str is another good example.
The %'s are kind of random if you ask me, because it is a % of what? 4.7% bias is defined how exactly...
or the one that puzzles me the most is the % next to the mean
mean = 104.7% .... ??

but yeah not trying to butcher that plot, I think that the idea behind it is still worthy, and there could be more applications that we do not know yet

[jsmariegaard]

But maybe it belongs more generally to the SkillTable than just the skill table next to the scatter plot... ?

[stkistner]

I agree that the (unbiased) str is another good example. The %'s are kind of random if you ask me, because it is a % of what? 4.7% bias is defined how exactly... or the one that puzzles me the most is the % next to the meanmean = 104.7% .... ??

The percentages are relative to the observation mean. I do not care that much for them, but can be useful still.

But maybe it belongs more generally to the SkillTable than just the skill table next to the scatter plot... ?

That is the longer (and better) option, but it will likely require some like an auxiliary metrics class / attribute. I'm not 100% what the solution would look like. If you think this is work pursuing we can look into it.

[daniel-caichac-DHI]

How would this work then, if I have a skill called for instance PR and the value is 1.17, how would you add the text on the right as you did in your example picture ?

[daniel-caichac-DHI]

as in this

[stkistner]

The metric output is

str=f"{pr:.2f} (N={n_joint}"

[daniel-caichac-DHI]

I mean... it works, but I guess it means that when creating a metric (eg BIAS) if I wanted something special (say BIAS_str)
instead of

def bias(obs: ArrayLike, model: ArrayLike) -> Any:
r"""Bias (mean error)
$$
bias=\frac{1}{n}\sum_{i=1}^n (model_i - obs_i)
$$
Range: $(-\infty, \infty)$; Best: 0
"""
assert obs.size == model.size
return np.mean(model - obs)

the user would need to do

def bias_str(obs: ArrayLike, model: ArrayLike) -> Any:
r"""Bias (mean error)
$$
bias=\frac{1}{n}\sum_{i=1}^n (model_i - obs_i)
$$
Range: $(-\infty, \infty)$; Best: 0
"""
assert obs.size == model.size
bias = np.mean(model - obs)
return f'bias {bias:.2f} this is a bias string'

?
I see nothing wrong with the current code addition, so I am pro-approving it, if @ecomodeller agrees

[ecomodeller]

I am not sure I get this, let's book a meeting to come up with a pragmatic solution.

[stkistner]

The string option is added (bnothing new, compared to before) and confirmed working with plotly. Some basic tests added to confirm it is working. It doesn't always work well with units (as that will always follow the metric output, but the end user should take care of that.

Also added now dict mapping for skill_table to rename the display names:

[stkistner]

@daniel-caichac-DHI / @ecomodeller , any comments?

[daniel-caichac-DHI]

This is cool

Since I have struggled to add the skill Mean_model and Mean_obs as it was too long, but now it should fit well :)

[daniel-caichac-DHI]

I think it looks great so I will approve,
but I guess @ecomodeller should press merge, unless he has any comments

[ecomodeller]

The skill table next to the scatter plot is starting to get complex enough that it deserves to be able to be tested independent from the scatter method.

[stkistner]

@ecomodeller I've,

• removed the argument from scatter plot
• updated metric decorator to accept display_name - effectively replaces this dictionary argument added to scatter table.
• Updated examples in notebook
• scatter table now has its own dedicated tests

[daniel-caichac-DHI]

@ecomodeller could we merge this now? or u still have comments?