awslabs / gluonts

Probabilistic time series modeling in Python
https://ts.gluon.ai
Apache License 2.0
4.63k stars 755 forks source link

Deprecate `make_evaluation_predictions` #474

Open jaheba opened 4 years ago

jaheba commented 4 years ago

Instead of using make_evaluation_predictions predictors should know how to evaluate themselves:

make_evaluation_predictions(test_dataset=test_dataset, estimator=predictor)

# vs 

predictor.evaluate(test_dataset)
mbohlkeschneider commented 4 years ago

What would be the advantage?

Generally, I think evaluation code should be strictly separate from model (estimator/predictor) code, because they have different functions and should therefore be separated.

lostella commented 4 years ago

Generally, I think evaluation code should be strictly separate from model (estimator/predictor) code

I agree, but I guess the idea is to still keep the code separate by having the evaluate method in the base Predictor class.

@jaheba one observation: the way you put it

predictor.evaluate(test_dataset)

would be a replacement for backtest_metrics rather than make_evaluation_predictions.

lostella commented 4 years ago

I like this proposal because:

  1. One doesn't need additional imports.
  2. The evaluation is unambiguously done on the predictor object, and not on a predictor OR an estimator (in which case training needs to be done). If we want to keep the functionality on estimators, we can add the same method to the base Estimator class which will do the training and call the evaluation method of the resulting predictor.
  3. We could get rid of the backtest_metrics/make_evaluation_predictions duality by means of optional arguments to select whether to compute only predictions or also evaluation metrics.
lostella commented 4 years ago

@jaheba should we punt this to the 0.6 milestone?