Skip to content

Rework quantile scores#388

Merged
nikosbosse merged 94 commits intoscoringutils-reviewfrom
rework-quantile-scores
Nov 16, 2023
Merged

Rework quantile scores#388
nikosbosse merged 94 commits intoscoringutils-reviewfrom
rework-quantile-scores

Conversation

@nikosbosse
Copy link
Copy Markdown
Collaborator

@nikosbosse nikosbosse commented Nov 4, 2023

Update: I reworked the part about bias_quantile() again in #396. Maybe it would be easiest to just ignore quantile_bias() for now? I can also merge #396 into this one (although there are some updates in between) or I could replicate the changes in #396 here to make reviewing it a bit easier.

This PR

  • creates a version of the WIS that
    • accepts a quantile input
    • returns one single score for a forecast (comprising multiple predictive quantiles, see image below)
  • Creates a function that computes the WIS for a one-to-one relationship
    • Meaning that it multiple scores for one forecast, namely one score per quantile
    • This is the behaviour of score() at the moment where you get one score per quantile. After talking to Seb it's a bit unclear whether we want / need that.
    • So at the moment the function just sits around. I created an issue to decide what to do with it. Also the different output formats seem a bit crazy, so maybe there is no need to review it now until we made a decision how it fits in (can also remove it from this PR if you prefer).
    • Issue related to that is Decide how WIS should be computed and returned #386
  • Adds input checks for forecasts in an interval format (which we need to think about whether or not to support an in what way, again related to Decide how WIS should be computed and returned #386)
  • Update bias_quantile() to work with several observations instead of only one (accepting the same input formats as wis(). This makes use of a function bias_quantile_single() which ultimately could be replaced by just data.table code in a single function. In the spirit of not cramming to many things in here (which I appreciate I completely failed to do yet again) I created an issue for that Merge bias_quantile() and bias_quantile_single() #387
  • Update examples and tests

Next step in a future PR is then to update score.quantile() to work with these functions.

…p. Function is horribly named and we need to decide what to do with it
… quantile must be equal to the length of predictions if there is only one observation
…hly hoping this is a bit easier / cleaner...
… well by introducing a new function bias_quantile_single
…nstances where the lower bound of an interval is higher than an upper bound
@nikosbosse nikosbosse requested a review from seabbs November 4, 2023 09:54
@nikosbosse
Copy link
Copy Markdown
Collaborator Author

@jhellewell14 maybe this is not the most terrible PR to have a look at if you feel inclined

@seabbs
Copy link
Copy Markdown
Contributor

seabbs commented Nov 7, 2023

Do you want this reviewed @nikosbosse?

@nikosbosse
Copy link
Copy Markdown
Collaborator Author

Yes please!

Copy link
Copy Markdown
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

If you could clean this up into the actual proposal that would be good as trying to review across two PRs without sufficient information is very hard.

Base automatically changed from rework-quantile-to-interval-format to scoringutils-review November 15, 2023 14:31
nikosbosse and others added 12 commits November 15, 2023 15:36
First step towards reworking `score.scoringutils_quantile()` - adding coverage as a metric
Update `bias_quantile()` to work with vectors / matrices instead of data.table
Update functions to compute the absolute median
…uantile()2

Rework score.scoringutils quantile()2
Rework add coverage to work with raw forecasts
Add separate functions for wis components
Copy link
Copy Markdown
Contributor

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

I think this is effectively unreviewable in its current form and given where we are with the rest of the development we should merge and aim to improve the use of planning issues and modular PRs going forward.

@nikosbosse nikosbosse merged commit 4df4576 into scoringutils-review Nov 16, 2023
@nikosbosse nikosbosse deleted the rework-quantile-scores branch November 16, 2023 11:23
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