Skip to content

Rework add coverage to work with raw forecasts#426

Merged
nikosbosse merged 8 commits intoadd-coverage-deviationfrom
reword-add_coverage2
Nov 15, 2023
Merged

Rework add coverage to work with raw forecasts#426
nikosbosse merged 8 commits intoadd-coverage-deviationfrom
reword-add_coverage2

Conversation

@nikosbosse
Copy link
Copy Markdown
Collaborator

This function updates the add_coverage() function yet again. This time

  • The function add_coverage() now adds rowwise coverage values for interval coverage and quantile coverage
  • Previously failing tests for other functions were updated

@nikosbosse nikosbosse changed the base branch from main to expand-tests2 November 13, 2023 16:06
@nikosbosse nikosbosse requested a review from seabbs November 13, 2023 16:06
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 13, 2023

Codecov Report

❗ No coverage uploaded for pull request base (expand-tests2@0d42acc). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 698b3c7 differs from pull request most recent head 79fc533. Consider uploading reports for the commit 79fc533 to get more accurate results

@@               Coverage Diff                @@
##             expand-tests2     #426   +/-   ##
================================================
  Coverage                 ?   80.25%           
================================================
  Files                    ?       20           
  Lines                    ?     1702           
  Branches                 ?        0           
================================================
  Hits                     ?     1366           
  Misses                   ?      336           
  Partials                 ?        0           

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@nikosbosse nikosbosse changed the title Reword add coverage to work with raw forecasts Rework add coverage to work with raw forecasts Nov 14, 2023
data[, quantile_coverage_deviation := quantile_coverage - quantile]

# reset column order
new_metrics <- c("interval_coverage", "interval_coverage_deviation",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you use the metrices functions vs duplicating here to make the code cleaner?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

At the moment not really - the thing is that the metrics functions do a many-to-one mapping (i.e. you get one coverage value per forecast (which comprises several quantiles)), whereas this is one-to-one, i.e. one value per quantile / interval.
We need to think again about how to handle one-to-one functions that exist currently and what to do with them: #451

example_quantile %>%
set_forecast_unit(c("location", "target_end_date", "target_type", "horizon", "model")) %>%
validate() %>%
add_coverage() %>%
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if its in the default metrics list what does this actually do? Is it going to be clear to users why it isn't part of score?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updating the readme is on the list: #439

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.

Linting issues otherwise fine.

Its not clear to me what the workflow is with add_coverage vs the coverage metrics in score or how that is going to be communicated to the user.

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.

Linting issues otherwise fine.

Its not clear to me what the workflow is with add_coverage vs the coverage metrics in score or how that is going to be communicated to the user.

Base automatically changed from expand-tests2 to add-coverage-deviation November 15, 2023 15:20
@nikosbosse nikosbosse merged commit e3fcec3 into add-coverage-deviation Nov 15, 2023
@nikosbosse nikosbosse deleted the reword-add_coverage2 branch November 15, 2023 17:04
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