-
Notifications
You must be signed in to change notification settings - Fork 912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/metrics quantiles #2530
Feat/metrics quantiles #2530
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2530 +/- ##
==========================================
+ Coverage 93.77% 93.84% +0.07%
==========================================
Files 139 139
Lines 14765 14841 +76
==========================================
+ Hits 13846 13928 +82
+ Misses 919 913 -6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job, I can feel the conformal prediction getting closer and closer...
Some comments to simplify a bit some part of the code, also asking for some clarification.
darts/metrics/metrics.py
Outdated
raise_log( | ||
ValueError( | ||
"Metric output must have 1 dimension for aggregated metrics (e.g. `mae()`, ...), " | ||
"or 2 dimension for time dependent metrics (e.g. `ae()`, ...)" | ||
), | ||
logger=logger, | ||
) | ||
elif len(vals.shape) == 1: | ||
elif len(vals.shape) == 2: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we assume that the case vals.shape == 1
can never happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could assume that the if it's 1dim then it must be the components axis. But I wonder it's better to raise more errors here to make sure we actually get the expected shapes 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Checklist before merging this PR:
Fixes #2527.
Summary
q
, either from probabilistic predictions or predicted quantiles.TimeSeries.shape
to get the shape of the time seriesmiw
(Mean Interval Width, time aggregated) andiw
(Interval Width, per time step / non-aggregated) which compute the width of quantile intervalsq_intervals
(expected to be a tuple or sequence of tuples with (lower quantile, upper quantile).