New Peak Ratio #435
New Peak Ratio #435
Conversation
|
I'm trying to understand the parameters of the I think the It is also interesting to understand how the default values for these parameters were chosen.
|
|
What a pro. I'm amazed. As for the parameters, I guess we can just reference MIke EVA manual https://manuals.mikepoweredbydhi.help//2024/General/EVA_UserGuide.pdf |
|
@ecomodeller don't forget about the peasants |
daniel-caichac-DHI
requested review from
ecomodeller
and removed request for
jsmariegaard
|
all comments addressed |
| df_filter["Maximum"] = df_filter.max(axis=1) | ||
| df_filter.sort_values(by="Maximum", ascending=False, inplace=True) | ||
| # Finally we do the selection of the N- largest peaks from either model or measured | ||
| df_filter = df_filter.iloc[0:top_n_peaks, :] |
There was a problem hiding this comment.
top_n_peaks has to be integer for this to work. This is not always the case.
There was a problem hiding this comment.
@daniel-caichac-DHI did you take a look at this?
| @@ -543,20 +544,20 @@ def peak_ratio( | |||
| obs: pd.Series, | |||
| model: np.ndarray, | |||
| inter_event_level: float = 0.7, | |||
| AAP: int = 2, | |||
| inter_event_time="36h", | |||
| AAP: Union[int, float] = 2, | |||
There was a problem hiding this comment.
Does it make sense to use integer?
| AAP: Union[int, float] = 2, | |
| AAP: float = 2.0, |
Hi.
After a long discussion we decided to update the peak selection in the Peak Ratio.
Small change but has some implications in the results. Also the number of times the user gets a NaN (no joint events) is much lower now (still can happen with very short time series with no evident peaks).
I had to obviously update the tests results of PR as the expected values now change.
Aligns with what was done here:
https://github.com/DHI/potpy/pull/39