[Bug] ModelTrainer.maybe_get_best_weights() does not deal properly with negative evaluation scores

[jan1854]

mbrl-lib/mbrl/models/model_trainer.py

Line 220 in 621832f

improvement = (best_val_score - val_score) / best_val_score

The above calculation of the relative improvement of the evaluation score in ModelTrainer seems to be wrong for negative evaluation scores. This can be fixed by adding a torch.abs() around the divisor.

Steps to reproduce

import torch
from mbrl.models import ModelTrainer, GaussianMLP

dummy = GaussianMLP(1, 1, "cpu")
model_trainer = ModelTrainer(dummy)
previous_eval_value = torch.tensor(-1.0)
current_eval_value = torch.tensor(-10.0)
print(model_trainer.maybe_get_best_weights(previous_eval_value, current_eval_value))

Observed Results

model_trainer.maybe_get_best_weights() returns None, which should indicate that the evaluation value did not improve from previous_eval_value to current_eval_value.

Expected Results

The relative improvement from previous_eval_value to current_eval_value is 900%. Thus, model_trainer.maybe_get_best_weights() should return the parameters of the model, which would indicate that the evaluation value improved.

[jan1854]

While we are on the topic of ModelTrainer, it would be nice if the threshold for improvement could be specified when calling ModelTrainer.train(). Right now threshold is a parameter of ModelTrainer.maybe_get_best_weights(), but not of ModelTrainer.train(). Since different applications deal with different scales of evaluation scores (and relative improvements of these scores), it would be nice to have a bit more flexibility here.

[luisenp]

Good point, I never considered this particular case. Do you want to submit a pull request? You've been reporting bugs/fixes for a while, might as well get some contribution credit :)