mrdbourke / tensorflow-deep-learning

All course materials for the Zero to Mastery Deep Learning with TensorFlow course.
https://dbourke.link/ZTMTFcourse
MIT License
5.14k stars 2.53k forks source link

Error in Time Series model_10 #340

Open ashwinshetgaonkar opened 2 years ago

ashwinshetgaonkar commented 2 years ago

mae value did not match when evaluated using mode_10.evaluate() and using evaluate_time_series() as tf.squeeze was not performed on the predictions(turkey_preds). we have used mae as the loss function, so evaluate() returned 638.3 which is the correct value not 17144.766 image

Nikhil-Nandam commented 2 years ago

Rightly pointed out!! Kudos for noticing the minor change!!! 👏

wr0ngc0degen commented 1 year ago

@mrdbourke I think this is a major error since it makes BS all the "Why forecasting is BS" section because if you recalculate the metrics correctly you might potentially see them even improving a bit against the model_1_results or at least not to differ that much. of course it depends on the metric - we know that mse is very sensitive to outliers and mape is known to penalize overforecasts heavier than underforecasts ie should've you decided to modify the last value in timeseries by multiplying instead of dividing, mape wouldn't've changed so much.

also, I disagree with @ashwinshetgaonkar in the root cause. tf.squeeze was performed on turkey_preds inside the make_preds function. but tf.squeeze wasn't applied to y_test. And you missed this point because since model 1 you'd modified the evaluate_preds function to take multidimensional inputs.

Just another though that would've helped to spot the error. If you think about it, MASE shouldn't've changed its value at all with the turkey data. Because MASE "compares" model's predictions with naïve model's predictions. And of course naïve model also fails on turkey datapoint, so the value of MASE shouldn't change, it should stay around 1 (the way it's calculated).