-
Notifications
You must be signed in to change notification settings - Fork 863
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
Fix/metrics #129
Fix/metrics #129
Changes from 10 commits
aac629d
845709a
18f6ccb
dd6c9b1
61ec34f
40adc66
b9247bb
dc18afc
2e1c04f
46fffa8
c217082
e997440
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,9 +9,11 @@ | |
|
||
class MetricsTestCase(unittest.TestCase): | ||
|
||
pd_train = pd.Series(np.sin(np.pi * np.arange(31) / 4) + 1, index=pd.date_range('20121201', '20121231')) | ||
pd_series1 = pd.Series(range(10), index=pd.date_range('20130101', '20130110')) | ||
pd_series2 = pd.Series(np.random.rand(10) * 10 + 1, index=pd.date_range('20130101', '20130110')) | ||
pd_series3 = pd.Series(np.sin(np.pi * np.arange(20) / 4) + 1, index=pd.date_range('20130101', '20130120')) | ||
series_train = TimeSeries.from_series(pd_train) | ||
series1: TimeSeries = TimeSeries.from_series(pd_series1) | ||
pd_series1[:] = pd_series1.mean() | ||
series0: TimeSeries = TimeSeries.from_series(pd_series1) | ||
|
@@ -30,15 +32,22 @@ def test_zero(self): | |
with self.assertRaises(ValueError): | ||
metrics.mape(self.series1, self.series1) | ||
|
||
with self.assertRaises(ValueError): | ||
metrics.smape(self.series1, self.series1) | ||
|
||
Comment on lines
+35
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly to my comment on @pennfranc PR I believe that we should split these tests and try to keep a single test for a single assert or at least a single test for a single failure reason. (each test method name should describe what it is testing). The problem with the current tests is that if mape is failing we have no idea if it is only mape or other methods will also fail like smape also I believe the reason for failing is not very clear each time. Let me know what you think :) |
||
with self.assertRaises(ValueError): | ||
metrics.mape(self.series12, self.series12) | ||
|
||
with self.assertRaises(ValueError): | ||
metrics.smape(self.series12, self.series12) | ||
|
||
with self.assertRaises(ValueError): | ||
metrics.ope(self.series1 - self.series1.pd_series().mean(), self.series1 - self.series1.pd_series().mean()) | ||
|
||
def test_same(self): | ||
self.assertEqual(metrics.mape(self.series1 + 1, self.series1 + 1), 0) | ||
self.assertEqual(metrics.mase(self.series1 + 1, self.series1 + 1, 1), 0) | ||
self.assertEqual(metrics.smape(self.series1 + 1, self.series1 + 1), 0) | ||
self.assertEqual(metrics.mase(self.series1 + 1, self.series1 + 1, self.series_train, 1), 0) | ||
self.assertEqual(metrics.marre(self.series1 + 1, self.series1 + 1), 0) | ||
self.assertEqual(metrics.r2_score(self.series1 + 1, self.series1 + 1), 1) | ||
self.assertEqual(metrics.ope(self.series1 + 1, self.series1 + 1), 0) | ||
|
@@ -47,15 +56,15 @@ def helper_test_shape_equality(self, metric): | |
self.assertAlmostEqual(metric(self.series12, self.series21), | ||
metric(self.series1.append(self.series2b), self.series2.append(self.series1b))) | ||
|
||
def helper_test_multivariate_duplication_equality(self, metric): | ||
def helper_test_multivariate_duplication_equality(self, metric, **kwargs): | ||
series11 = self.series1.stack(self.series1) + 1 | ||
series22 = self.series2.stack(self.series2) | ||
series33 = self.series3.stack(self.series3) | ||
self.assertAlmostEqual(metric(series11, series22), metric(self.series1 + 1, self.series2)) | ||
self.assertAlmostEqual(metric(series11, series33), metric(self.series1 + 1, self.series3)) | ||
self.assertAlmostEqual(metric(series22, series33), metric(self.series2, self.series3)) | ||
self.assertAlmostEqual(metric(series22, series33, reduction=(lambda x: x[0])), | ||
metric(self.series2, self.series3, reduction=(lambda x: x[0]))) | ||
self.assertAlmostEqual(metric(series11, series22, **kwargs), metric(self.series1 + 1, self.series2, **kwargs)) | ||
self.assertAlmostEqual(metric(series11, series33, **kwargs), metric(self.series1 + 1, self.series3, **kwargs)) | ||
self.assertAlmostEqual(metric(series22, series33, **kwargs), metric(self.series2, self.series3, **kwargs)) | ||
self.assertAlmostEqual(metric(series22, series33, reduction=(lambda x: x[0]), **kwargs), | ||
metric(self.series2, self.series3, reduction=(lambda x: x[0]), **kwargs)) | ||
|
||
def test_r2(self): | ||
from sklearn.metrics import r2_score | ||
|
@@ -71,7 +80,7 @@ def test_marre(self): | |
|
||
def test_season(self): | ||
with self.assertRaises(ValueError): | ||
metrics.mase(self.series3, self.series3 * 1.3, 8) | ||
metrics.mase(self.series3, self.series3 * 1.3, self.series_train, 8) | ||
|
||
def test_mse(self): | ||
self.helper_test_shape_equality(metrics.mse) | ||
|
@@ -94,8 +103,14 @@ def test_coefficient_of_variation(self): | |
def test_mape(self): | ||
self.helper_test_multivariate_duplication_equality(metrics.mape) | ||
|
||
def test_smape(self): | ||
self.helper_test_multivariate_duplication_equality(metrics.smape) | ||
|
||
def test_mase(self): | ||
self.helper_test_multivariate_duplication_equality(metrics.mase) | ||
self.helper_test_multivariate_duplication_equality(metrics.mase, insample=self.series_train) | ||
|
||
with self.assertRaises(ValueError): | ||
metrics.mase(self.series1, self.series2, self.series3, 1) | ||
|
||
def test_ope(self): | ||
self.helper_test_multivariate_duplication_equality(metrics.ope) | ||
|
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.