Add quantile loss metric

[JanFidor]

Fixes #1367

Summary

Add quantile loss metric with tests

I found an already existing implementation of quantile loss here:

darts/darts/utils/likelihood_models.py

Line 1121 in e1c8d34

errors = target.unsqueeze(-1) - model_output

and used it for refactoring. Substituting that implementation with the new metric function would definitely be in line with DRY, but for now it doesn't support CUDA, while the QuantileRegression does. I'm not certain whether it actually needs it or if it was just an implementation quirk, so I left it as is for now. I could add support for multiple tau values as a parameter and returning their mean loss.

Other Information

While we're at the topic of the QuantileRegression , while browsing it I stumbled upon this line:

darts/darts/utils/likelihood_models.py

Line 1126 in e1c8d34

return losses.sum(dim=dim_q).mean()

I'll be honest I've been looking at it for quite a while now and I just can't wrap my head around why it takes that sum before returning the mean. It would be penalizing the amount of quantiles the model optimizes for, which would kind of work as a regularization technique. Also if we have quantiles q and 1 - q we optimize for, they negate each other and give us 2 instances with quantile 0.5, so we could try to skip those unnecessary operations, I'm not certain if the speed-up would be meaningful though. Please let me know what you think

[madtoinou]

LGTM.

I don't have enough experience to comment the line you highlighted in darts/darts/utils/likelihood_models.py, I'll try to think more about it and circle back to you.

[JanFidor]

In the last commit I used simple mean across all values instead of the previously used reduction param, because it wasn't passing

darts/darts/tests/metrics/test_metrics.py

Line 112 in e1c8d34

def helper_test_multivariate_duplication_equality(self, metric, **kwargs):

[madtoinou]

LGTM, the reduction argument is actually supposed to be used when reducing the metrics for multivariate ts (thanks to the decorator) and should not be used inside the metric itself.

[JanFidor]

LGTM.

I don't have enough experience to comment the line you highlighted in darts/darts/utils/likelihood_models.py, I'll try to think more about it and circle back to you.

I mentioned it because I managed to stumble into it and it's somewhat connected to the PR. It's nothing crucial for now, so it might be a good idea to put together a separate issue for it to keep the PR small and focused on just the metric, let me know what you think

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.04 ⚠️

Comparison is base (7abb872) 94.09% compared to head (170e59f) 94.05%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1559      +/-   ##
==========================================
- Coverage   94.09%   94.05%   -0.04%     
==========================================
  Files         125      125              
  Lines       11185    11182       -3     
==========================================
- Hits        10524    10517       -7     
- Misses        661      665       +4     

Impacted Files	Coverage Δ
darts/metrics/__init__.py	100.00% <ø> (ø)
darts/metrics/metrics.py	100.00% <100.00%> (ø)
darts/utils/data/tabularization.py	99.27% <0.00%> (-0.73%)	⬇️
darts/timeseries.py	92.28% <0.00%> (-0.22%)	⬇️
darts/ad/anomaly_model/filtering_am.py	91.93% <0.00%> (-0.13%)	⬇️
...arts/models/forecasting/torch_forecasting_model.py	89.88% <0.00%> (-0.04%)	⬇️
darts/models/forecasting/block_rnn_model.py	98.24% <0.00%> (-0.04%)	⬇️
darts/models/forecasting/nhits.py	99.27% <0.00%> (-0.01%)	⬇️
darts/datasets/__init__.py	100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[hrzn]

LGTM, only small comments.

[hrzn]

@JanFidor thanks for the PR!
Related to your comment about the other implementation in likelihood_models.py. This other implementation is in PyTorch (and supporting CUDA) because it's directly used as a loss function for training the neural networks models (when the likelihood is set to "quantile").
In the case of the metrics, it's better to keep a Numpy implementation only, as you did.

Regarding the operation

darts/darts/utils/likelihood_models.py

Line 1126 in e1c8d34

return losses.sum(dim=dim_q).mean()

I would have to dive into it a little more, but I think this is intended. We are first summing the losses over the last dimensions, so that the final loss for each sample is the sum of the losses for each quantile; note that when training the gradients will flow backward on this sum and be redistributed to the individual "quantile heads" of the neural network, in order to tune them to predict the individual quantiles correctly. The last mean() call is simply to take the mean over the batch dimension. So at first sight nothing there looks suspicious to me, but I might be missing something of course.

Finally, I think your averaging in Numpy over the time dimension is correct, as we want to return the mean quantile loss (over time).

[JanFidor]

Thanks for the explanation, I wasn't looking at the big picture with back-prop, it makes sense now

[JanFidor]

Hmmm, I think macos failure might be unrelated to the code as ubuntu passes, please let me know if there's something I should change or if the CI's flaky

[hrzn]

💯

[JanFidor]

Thanks for the reviews @madtoinou @hrzn !