pySTEPS / pysteps

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

Return total scaled volume in SAL metric #345

Closed EsmailGhaemi closed 7 months ago

EsmailGhaemi commented 8 months ago

Fixed #344 (S parameter edited, L parameter modified for dealing with nan values)

Now the scaled volume function directly returns total scaled volume (instead of scaled voulme for each object). Additionally, for the weighted distance function, np.nansum() is used to avoid the effect of nan values.

codecov[bot] commented 7 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (c2f8320) 83.34% compared to head (a2b6761) 83.34%.

Files Patch % Lines
pysteps/verification/salscores.py 94.73% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #345 +/- ## ======================================= Coverage 83.34% 83.34% ======================================= Files 161 161 Lines 12356 12364 +8 ======================================= + Hits 10298 10305 +7 - Misses 2058 2059 +1 ``` | [Flag](https://app.codecov.io/gh/pySTEPS/pysteps/pull/345/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pySTEPS) | Coverage Δ | | |---|---|---| | [unit_tests](https://app.codecov.io/gh/pySTEPS/pysteps/pull/345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pySTEPS) | `83.34% <94.73%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pySTEPS#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dnerini commented 7 months ago

Thanks @EsmailGhaemi for the changes. Could you just spend few more words on what you did? It's not obvious to me what you changed, why, and what kind of impact we should expect. This will help to describe the changes in the release notes.

EsmailGhaemi commented 7 months ago

Yes, of course. In the previous version, the scaled volume was calculated for each object separately, and then in sal_srtucture function we summed all of them. However, when calculating total volume (V_R in equation 8, Wernli et al., 2008), we should multiply each object's scaled volume (v_n) to the amount of precipitation for that object (R_n) as well. Now the scaled volume function directly returns total scaled volume (instead of scaled voulme for each object). Additionally, for the weighted distance function, np.nansum() is used to avoid the effect of nan values.

dnerini commented 7 months ago

Brilliant, thanks, I've update the description above.