search

pySTEPS / pysteps

Python framework for short-term ensemble prediction systems.
https://pysteps.github.io/
BSD 3-Clause "New" or "Revised" License
466 stars 168 forks source link

location parameter L2 in the SAL verification #321

Closed MatildaHallerstig closed 1 year ago

MatildaHallerstig commented 1 year ago

In the function _sal_l2_param in pysteps.verification.salscores, obs_r and forc_r is obtained by multiplying output from _sal_weighted_distance with the meanvalue of model field. Why this multiplication? It makes it possible for l2 to exceed 1, which is different from what is said in the docstring. The source article, referred to as WPHF2008 in the docstring does not multiply with the model field mean value when they calculate obs_r and forc_r.

Best,

Matilda

dnerini commented 1 year ago

need to ping @EsmailGhaemi on this

EsmailGhaemi commented 1 year ago

Hi Matilda,

As you mentioned, there is no need for the multiplication factor. Thanks for sharing this issue. Best, Esmail

dnerini commented 1 year ago

@EsmailGhaemi thanks for your inputs, so we should simply get rid of it? Any idea why we introduced it in the first place?

It'd great if you could submit a pull request with the fixes needed for this, would that be possible?

EsmailGhaemi commented 1 year ago

We only get rid of the mean value as a multiplication factor for L2 parameter. It was a misunderstanding of the formula from my side.

dnerini commented 1 year ago

Very good, thanks a lot @EsmailGhaemi, I'll have a look asap at your PR.

@MatildaHallerstig could you have a quick look too? Does it address the issue you've reported?

MatildaHallerstig commented 1 year ago

@dnerini yes, the change fixes the issue. Thank you for addressing it!