Skip to content

Implement mean min max for impact forecast and hazard forecast#1187

Merged
peanutfun merged 34 commits intoforecast-classfrom
implement_mean_min_max
Dec 10, 2025
Merged

Implement mean min max for impact forecast and hazard forecast#1187
peanutfun merged 34 commits intoforecast-classfrom
implement_mean_min_max

Conversation

@luseverin
Copy link
Collaborator

@luseverin luseverin commented Dec 9, 2025

Changes proposed in this PR:

  • Implement mean, min, and max for impact forecast

Missing:

  • implement mean, min and max for hazard forecast

This PR fixes #1160

PR Author Checklist

PR Reviewer Checklist

luseverin and others added 23 commits December 8, 2025 15:10
@luseverin luseverin marked this pull request as draft December 9, 2025 16:15
@chahank
Copy link
Member

chahank commented Dec 9, 2025

Probably you need to merge the forecast branch into this one. There are changes in the impact calc that do not belong here I think.


@property
def at_event(self):
LOGGER.warning("at_event for forecasts is not yet implemented.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is not true. At event is defined for the forecast.

red_frequency = np.array([0])
return red_event_id, red_event_name, red_date, red_frequency

def min(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the goal not to reduce along a specified dimension? I am no sure to see the value of getting the minimum impact accross all exposure points, all lead times, and all ensemble members.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should reduce along the event dimension. Then we can compute the statistics over the member and/or lead time with the appropriate use of the select function. Does that makes sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me this method is exposed to the user, so it should directly give back a useful/meaningful result. So I would make the exposed method directly clear. Either over members, or over lead times (or both if the user for some reason wants it). Or make it private if the user is not supposed to use it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chahank We first implement the complete reductions, then we implement reduction over specific dimensions in #1163. This is easier from an implementation perspective, because the latter requires selection and concatenation to work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I would just make the complete reduction methods above private then.

@luseverin luseverin marked this pull request as ready for review December 10, 2025 10:27
@peanutfun peanutfun merged commit 2b9b388 into forecast-class Dec 10, 2025
10 of 11 checks passed
@emanuel-schmid emanuel-schmid deleted the implement_mean_min_max branch December 11, 2025 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants