Alternative implementation of the scatter index #142
Conversation
|
@daniel-caichac-DHI could you help to test by comparing it to your internal implementation of SI? |
|
I compared the results and I get the same, so at least the math is ok. |
fmskill/metrics.py
Outdated
| \\sqrt {\frac{1}{n} \\sum_{i=1}^n \\left( (model_i - \\overline {model}) - (obs_i - \\overline {obs}) \\right)^2 | ||
| {\frac{1}{n} \\sum_{i=1}^n obs_i^2}} | ||
|
|
||
| Range: [0, 100]; Best: 0 |
There was a problem hiding this comment.
I don't think the range and the best value is correct🤔
There was a problem hiding this comment.
I guess that the value is between 0 and 1, but the best value should be 0 (if BIAS=0 and model=measurement, which would a Perfect Model , then the Numerator is 0)
There was a problem hiding this comment.
I agree that the best value is 0, but the range is
There was a problem hiding this comment.
I see, the denominator should be the mean of the abs (x) and then the range could not be negative.
There was a problem hiding this comment.
alright done, I checked against the other implementation and it gives the same result.
I updated the test as well ( I had no idea tests could fail on the docstring as well! )
In this branch the new SI implementation becomes the default, overriding the old one
There was a problem hiding this comment.
alright done, I checked against the other implementation and it gives the same result. I updated the test as well ( I had no idea tests could fail on the docstring as well! ) In this branch the new SI implementation becomes the default, overriding the old one
Take a look at Doctest 🤯
There was a problem hiding this comment.
Now a question to both of you, this branch was uploaded by Jesper as a "draft", should we keep it like that? or should we merge to main? if so, what happens with the old SI implementation? select section + delete ? or we leave it as it is now (renamed as scatter_index2) and erase it in the future?
There was a problem hiding this comment.
@daniel-caichac-DHI I think you are the only using this metric. (it doesn't make sense to anyone else😉)
📢 If anyone else is using this metric and wants to keep the other formulation, now is the time to speak up, otherwise it will be gone!
There was a problem hiding this comment.
everyone I know is using this metric!
I will remove the draft then and move it into the soon to be merged pipeline
Closes #137