custom metrics in skill table

[daniel-caichac-DHI]

Hi,

As was discussed here #226 I needed to have some custom metrics in the skill table, so I implemented an option to "add metrics" on demand, just by appending them to the sets DEFINED_METRICS and METRICS_WITH_DIMENSION if that is the case.

The code that was failing in issue/226 now works like this (I added two metrics one with units and one without for this example)

• I added some tests as well

[ecomodeller]

In your example you refer to the custom metrics as python functions. Is it a requirement that the function is in scope, i.e. global scope or can you also refer to it by string, e.g. 'pr'?

[daniel-caichac-DHI]

I have it implemented as python function, since that is the only way I can think of to add a custom metric.
And then I am looking at the name of the function when adding it to the set. For instance if I define:
def pr(obs,model):
return mean(obs/model)

then when adding pr to the defined metrics I actually add pr.__name__
which I understand is the same that is being done here

modelskill/modelskill/metrics.py

Line 605 in 6dc27ec

if callable(getattr(sys.modules[__name__], func)) and not func.startswith("_")

I have not actually tried to then just use 'pr' as a str

[daniel-caichac-DHI]

Ok I tried with calling a custom metric as a string and does not work, but I think that it is not too terrible. It is a reasonable compromise, considering the number of times a custom metrics needs to be actually used in the scatter plot skill table.
If the number of times a custom metric is needed is too large, then a Pull Request is warranted where that metric is actually added to metrics.py.
IMO

[jsmariegaard]

I think it's great that you guys are working on custom metrics and we probably need that. Great. But if you have some metrics that you use over and over again I also think we should add these to the metrics module (in another PR).

[daniel-caichac-DHI]

@jsmariegaard: that's exactly what I suggested in my last comment!
If the user finds him/herself reusing a metric time and time again, it is better to add it to metrics.py, but it is also important to have this feature of custom metrics available :)

As of now, this looks good to me, should be merged!

[jsmariegaard]

@jsmariegaard: that's exactly what I suggested in my last comment! If the user finds him/herself reusing a metric time and time again, it is better to add it to metrics.py, but it is also important to have this feature of custom metrics available :)

TL;DR 😳

But @daniel-caichac-DHI why didn't you make that PR with your favorite metrics then 🤔?

[jsmariegaard]

Is it peak-ratio and explained variance?

[daniel-caichac-DHI]

I will do it, after this PR is merged, otherwise the "custom metric" will not be added! Which I also need for other projects

[ecomodeller]

Is it peak-ratio and explained variance?

Explained variance == $R^2$ ?

[jsmariegaard]

Looks great

[daniel-caichac-DHI]

Is it peak-ratio and explained variance?

Explained variance == R2 ?

It gives very different results to R2 as is implemented in modelskill

[ecomodeller]

Is it peak-ratio and explained variance?

Explained variance == R2 ?

It gives very different results to R2 as is implemented in modelskill

R2 is often referred to as explained variance which is why I think it is pretty confusing with another metric with a similar name giving other results, do you have any authoritative reference other than the screenshot🤨?