From 4aa6ce6570809cb9ad9ab0cfdde65fd98656c9fe Mon Sep 17 00:00:00 2001 From: "Leandro G. Almeida" Date: Mon, 5 Apr 2021 17:25:07 -0700 Subject: [PATCH 1/8] added more tests and make sure merger doesnt affect model type --- src/whylogs/core/metrics/model_metrics.py | 15 +++-- tests/unit/core/metrics/test_model_metrics.py | 57 +++++++++++++++++-- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/whylogs/core/metrics/model_metrics.py b/src/whylogs/core/metrics/model_metrics.py index 5837384e96..1ebea4c110 100644 --- a/src/whylogs/core/metrics/model_metrics.py +++ b/src/whylogs/core/metrics/model_metrics.py @@ -99,17 +99,22 @@ def merge(self, other): if other.confusion_matrix is None and other.regression_metrics is None: # TODO: return a copy instead return self + if self.confusion_matrix is None and self.regression_metrics is None: return other - if self.model_type is None or other.model_type is None: - model_type = ModelType.UNKNOWN - elif other.model_type != self.model_type: - model_type = ModelType.UNKNOWN + if (self.model_type not in (ModelType.REGRESSION, ModelType.CLASSIFICATION)): + if other.model_type in (ModelType.REGRESSION, ModelType.CLASSIFICATION): + model_type = other.model_type + + elif other.model_type not in (ModelType.REGRESSION, ModelType.CLASSIFICATION): + if self.model_type in (ModelType.REGRESSION, ModelType.CLASSIFICATION): + model_type = self.model_type else: model_type = self.model_type return ModelMetrics( confusion_matrix=self.confusion_matrix.merge(other.confusion_matrix) if self.confusion_matrix else None, - regression_metrics=self.regression_metrics.merge(other.regression_metrics)if self.regression_metrics else None, + regression_metrics=self.regression_metrics.merge( + other.regression_metrics)if self.regression_metrics else None, model_type=model_type) diff --git a/tests/unit/core/metrics/test_model_metrics.py b/tests/unit/core/metrics/test_model_metrics.py index 3384dcf14e..0be59bb3e2 100644 --- a/tests/unit/core/metrics/test_model_metrics.py +++ b/tests/unit/core/metrics/test_model_metrics.py @@ -26,7 +26,7 @@ def tests_model_metrics(): jdx].floats.count == expected_1[idx][jdx] -def tests_model_metrics_to_protobuf(): +def tests_model_metrics_to_protobuf_classification(): mod_met = ModelMetrics(model_type=ModelType.CLASSIFICATION) targets_1 = ["cat", "dog", "pig"] @@ -39,7 +39,46 @@ def tests_model_metrics_to_protobuf(): message = mod_met.to_protobuf() - ModelMetrics.from_protobuf(message) + model_metrics = ModelMetrics.from_protobuf(message) + assert model_metrics.model_type == ModelType.CLASSIFICATION + assert model_metrics.confusion_matrix.labels == ["cat", "dog", "pig"] + + +def tests_no_metrics_to_protobuf_classification(): + mod_met = ModelMetrics(model_type=ModelType.CLASSIFICATION) + + + message = mod_met.to_protobuf() + + model_metrics = ModelMetrics.from_protobuf(message) + assert model_metrics.model_type == ModelType.CLASSIFICATION + +def tests_no_metrics_to_protobuf_regression():\ + + mod_met = ModelMetrics(model_type=ModelType.REGRESSION) + assert mod_met.model_type == ModelType.REGRESSION + message = mod_met.to_protobuf() + + + model_metrics = ModelMetrics.from_protobuf(message) + assert model_metrics.model_type == ModelType.REGRESSION + +def tests_model_metrics_to_protobuf_regression(): + + + regression_model = ModelMetrics(model_type=ModelType.REGRESSION) + + + targets_1 = [0.1, 0.3, 0.4] + predictions_1 = [0.5, 0.5, 0.5] + regression_model.compute_regression_metrics(predictions_1, targets_1) + regression_message = regression_model.to_protobuf() + model_metrics_from_message = ModelMetrics.from_protobuf(regression_message) + assert model_metrics_from_message.model_type == ModelType.REGRESSION + + + + def test_merge_none(): @@ -54,11 +93,19 @@ def test_merge_metrics_with_none_confusion_matrix(): new_metrics = metrics.merge(other) +def test_merge_metrics_model(): + metrics = ModelMetrics() + other = ModelMetrics() + other.regression_metrics = None + new_metrics = metrics.merge(other) + + def test_merge_metrics_with_none_regression_matrix(): metrics = ModelMetrics() other = ModelMetrics() other.regression_metrics = None - new_metrics= metrics.merge(other) + new_metrics = metrics.merge(other) + def test_merge_metrics_with_none_confusion_matrix(): metrics = ModelMetrics() @@ -68,9 +115,9 @@ def test_merge_metrics_with_none_confusion_matrix(): new_metrics = metrics.merge(other) + def test_model_metrics_init(): reg_met = RegressionMetrics() - conf_ma= ConfusionMatrix() + conf_ma = ConfusionMatrix() with pytest.raises(NotImplementedError): metrics = ModelMetrics(confusion_matrix=conf_ma, regression_metrics=reg_met) - From f487c8511403981e4f34c1d7460c4d5346aea558 Mon Sep 17 00:00:00 2001 From: "Leandro G. Almeida" Date: Mon, 5 Apr 2021 17:43:07 -0700 Subject: [PATCH 2/8] model type is set regardless of metric presence --- src/whylogs/core/metrics/model_metrics.py | 10 +++------- src/whylogs/core/metrics/regression_metrics.py | 2 ++ tests/unit/core/metrics/test_model_metrics.py | 15 ++++++--------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/whylogs/core/metrics/model_metrics.py b/src/whylogs/core/metrics/model_metrics.py index 1ebea4c110..7d1eb379d4 100644 --- a/src/whylogs/core/metrics/model_metrics.py +++ b/src/whylogs/core/metrics/model_metrics.py @@ -96,13 +96,8 @@ def merge(self, other): """ if other is None: return self - if other.confusion_matrix is None and other.regression_metrics is None: - # TODO: return a copy instead - return self - - if self.confusion_matrix is None and self.regression_metrics is None: - return other - + + model_type =ModelType.UNKNOWN if (self.model_type not in (ModelType.REGRESSION, ModelType.CLASSIFICATION)): if other.model_type in (ModelType.REGRESSION, ModelType.CLASSIFICATION): model_type = other.model_type @@ -113,6 +108,7 @@ def merge(self, other): else: model_type = self.model_type + return ModelMetrics( confusion_matrix=self.confusion_matrix.merge(other.confusion_matrix) if self.confusion_matrix else None, regression_metrics=self.regression_metrics.merge( diff --git a/src/whylogs/core/metrics/regression_metrics.py b/src/whylogs/core/metrics/regression_metrics.py index 95a98c199c..e93bb11ac1 100644 --- a/src/whylogs/core/metrics/regression_metrics.py +++ b/src/whylogs/core/metrics/regression_metrics.py @@ -74,6 +74,8 @@ def merge(self, other): Returns: RegressionMetrics: merged regression metrics """ + if other is None: + return self if self.count == 0: return other diff --git a/tests/unit/core/metrics/test_model_metrics.py b/tests/unit/core/metrics/test_model_metrics.py index 0be59bb3e2..d9bd83edb8 100644 --- a/tests/unit/core/metrics/test_model_metrics.py +++ b/tests/unit/core/metrics/test_model_metrics.py @@ -47,13 +47,13 @@ def tests_model_metrics_to_protobuf_classification(): def tests_no_metrics_to_protobuf_classification(): mod_met = ModelMetrics(model_type=ModelType.CLASSIFICATION) - + assert mod_met.model_type == ModelType.CLASSIFICATION message = mod_met.to_protobuf() model_metrics = ModelMetrics.from_protobuf(message) assert model_metrics.model_type == ModelType.CLASSIFICATION -def tests_no_metrics_to_protobuf_regression():\ +def tests_no_metrics_to_protobuf_regression(): mod_met = ModelMetrics(model_type=ModelType.REGRESSION) assert mod_met.model_type == ModelType.REGRESSION @@ -68,7 +68,6 @@ def tests_model_metrics_to_protobuf_regression(): regression_model = ModelMetrics(model_type=ModelType.REGRESSION) - targets_1 = [0.1, 0.3, 0.4] predictions_1 = [0.5, 0.5, 0.5] regression_model.compute_regression_metrics(predictions_1, targets_1) @@ -78,9 +77,6 @@ def tests_model_metrics_to_protobuf_regression(): - - - def test_merge_none(): metrics = ModelMetrics() metrics.merge(None) @@ -95,17 +91,18 @@ def test_merge_metrics_with_none_confusion_matrix(): def test_merge_metrics_model(): metrics = ModelMetrics() - other = ModelMetrics() + other = ModelMetrics(model_type=ModelType.REGRESSION) other.regression_metrics = None new_metrics = metrics.merge(other) + assert new_metrics.model_type==ModelType.REGRESSION def test_merge_metrics_with_none_regression_matrix(): metrics = ModelMetrics() - other = ModelMetrics() + other = ModelMetrics(model_type=ModelType.REGRESSION) other.regression_metrics = None new_metrics = metrics.merge(other) - + assert new_metrics.model_type==ModelType.REGRESSION def test_merge_metrics_with_none_confusion_matrix(): metrics = ModelMetrics() From e3c933627516d30cd4f52d9713bdb88fe9fe9878 Mon Sep 17 00:00:00 2001 From: "Leandro G. Almeida" Date: Tue, 6 Apr 2021 08:56:16 -0700 Subject: [PATCH 3/8] fix pep8 add anoter merge test --- src/whylogs/core/metrics/model_metrics.py | 6 +++--- tests/unit/core/metrics/test_model_metrics.py | 11 ++++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/whylogs/core/metrics/model_metrics.py b/src/whylogs/core/metrics/model_metrics.py index 7d1eb379d4..e9506d913b 100644 --- a/src/whylogs/core/metrics/model_metrics.py +++ b/src/whylogs/core/metrics/model_metrics.py @@ -96,8 +96,9 @@ def merge(self, other): """ if other is None: return self - - model_type =ModelType.UNKNOWN + + model_type = ModelType.UNKNOWN + if (self.model_type not in (ModelType.REGRESSION, ModelType.CLASSIFICATION)): if other.model_type in (ModelType.REGRESSION, ModelType.CLASSIFICATION): model_type = other.model_type @@ -108,7 +109,6 @@ def merge(self, other): else: model_type = self.model_type - return ModelMetrics( confusion_matrix=self.confusion_matrix.merge(other.confusion_matrix) if self.confusion_matrix else None, regression_metrics=self.regression_metrics.merge( diff --git a/tests/unit/core/metrics/test_model_metrics.py b/tests/unit/core/metrics/test_model_metrics.py index d9bd83edb8..ff2d235d57 100644 --- a/tests/unit/core/metrics/test_model_metrics.py +++ b/tests/unit/core/metrics/test_model_metrics.py @@ -79,7 +79,7 @@ def tests_model_metrics_to_protobuf_regression(): def test_merge_none(): metrics = ModelMetrics() - metrics.merge(None) + assert metrics.merge(None) == metrics def test_merge_metrics_with_none_confusion_matrix(): @@ -95,7 +95,15 @@ def test_merge_metrics_model(): other.regression_metrics = None new_metrics = metrics.merge(other) assert new_metrics.model_type==ModelType.REGRESSION + assert new_metrics.confusion_matrix is None + # keep initial model type during merge + metrics = ModelMetrics(model_type=ModelType.REGRESSION) + other = ModelMetrics(model_type=ModelType.CLASSIFICATION) + other.regression_metrics = None + new_metrics = metrics.merge(other) + assert new_metrics.model_type==ModelType.REGRESSION + assert new_metrics.confusion_matrix is None def test_merge_metrics_with_none_regression_matrix(): metrics = ModelMetrics() @@ -111,6 +119,7 @@ def test_merge_metrics_with_none_confusion_matrix(): other.regression_metrics = None new_metrics = metrics.merge(other) + assert new_metrics.model_type == ModelType.UNKNOWN def test_model_metrics_init(): From cbfb4c93c14e1cc2d489d1d5cb66b565d9423056 Mon Sep 17 00:00:00 2001 From: "Leandro G. Almeida" Date: Tue, 6 Apr 2021 09:00:27 -0700 Subject: [PATCH 4/8] =?UTF-8?q?Bump=20version:=200.4.4-dev0=20=E2=86=92=20?= =?UTF-8?q?0.4.5-dev0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .bumpversion.cfg | 2 +- docs/conf.py | 2 +- setup.cfg | 2 +- src/whylogs/_version.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index d6b05ceebf..bbebabc720 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.4.4-dev0 +current_version = 0.4.5-dev0 commit = True tag = False parse = (?P\d+)\.(?P\d+)\.(?P\d+)(\-(?P[a-z]+)(?P\d+))? diff --git a/docs/conf.py b/docs/conf.py index 2ce07541f3..4007d51760 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -101,7 +101,7 @@ # built documents. # # The short X.Y version. -version = "0.4.4-dev0" +version = "0.4.5-dev0" # The full version, including alpha/beta/rc tags. release = "" # Is set by calling `setup.py docs` diff --git a/setup.cfg b/setup.cfg index 0808212ed8..c204ebab54 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,7 +4,7 @@ [metadata] name = whylogs -version = 0.4.4-dev0 +version = 0.4.5-dev0 description = Profile and monitor your ML data pipeline end-to-end author = WhyLabs.ai author-email = support@whylabs.ai diff --git a/src/whylogs/_version.py b/src/whylogs/_version.py index 489d85aff6..bf17669e52 100644 --- a/src/whylogs/_version.py +++ b/src/whylogs/_version.py @@ -1,3 +1,3 @@ """WhyLabs version number.""" -__version__ = "0.4.4-dev0" +__version__ = "0.4.5-dev0" From da3bbb91080e15db153f8a276ffa1aad2f886f4a Mon Sep 17 00:00:00 2001 From: "Leandro G. Almeida" Date: Tue, 6 Apr 2021 09:16:52 -0700 Subject: [PATCH 5/8] Update tests/unit/core/metrics/test_model_metrics.py --- tests/unit/core/metrics/test_model_metrics.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/core/metrics/test_model_metrics.py b/tests/unit/core/metrics/test_model_metrics.py index ff2d235d57..e26658387a 100644 --- a/tests/unit/core/metrics/test_model_metrics.py +++ b/tests/unit/core/metrics/test_model_metrics.py @@ -65,7 +65,6 @@ def tests_no_metrics_to_protobuf_regression(): def tests_model_metrics_to_protobuf_regression(): - regression_model = ModelMetrics(model_type=ModelType.REGRESSION) targets_1 = [0.1, 0.3, 0.4] From 7b436401f7d589d82c1a1803837c0383315c34f4 Mon Sep 17 00:00:00 2001 From: "Leandro G. Almeida" Date: Tue, 6 Apr 2021 09:18:53 -0700 Subject: [PATCH 6/8] Update tests/unit/core/metrics/test_model_metrics.py --- tests/unit/core/metrics/test_model_metrics.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/core/metrics/test_model_metrics.py b/tests/unit/core/metrics/test_model_metrics.py index e26658387a..ce48b927da 100644 --- a/tests/unit/core/metrics/test_model_metrics.py +++ b/tests/unit/core/metrics/test_model_metrics.py @@ -64,7 +64,6 @@ def tests_no_metrics_to_protobuf_regression(): assert model_metrics.model_type == ModelType.REGRESSION def tests_model_metrics_to_protobuf_regression(): - regression_model = ModelMetrics(model_type=ModelType.REGRESSION) targets_1 = [0.1, 0.3, 0.4] From 904391bd735876ea8d4c3b54bcc7badc157516ec Mon Sep 17 00:00:00 2001 From: "Leandro G. Almeida" Date: Wed, 7 Apr 2021 19:41:37 -0700 Subject: [PATCH 7/8] =?UTF-8?q?Bump=20version:=200.4.5-dev0=20=E2=86=92=20?= =?UTF-8?q?0.4.5-dev1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .bumpversion.cfg | 2 +- docs/conf.py | 2 +- setup.cfg | 2 +- src/whylogs/_version.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index bbebabc720..73e51b3de2 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 0.4.5-dev0 +current_version = 0.4.5-dev1 commit = True tag = False parse = (?P\d+)\.(?P\d+)\.(?P\d+)(\-(?P[a-z]+)(?P\d+))? diff --git a/docs/conf.py b/docs/conf.py index 4007d51760..c9fa4c05f7 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -101,7 +101,7 @@ # built documents. # # The short X.Y version. -version = "0.4.5-dev0" +version = "0.4.5-dev1" # The full version, including alpha/beta/rc tags. release = "" # Is set by calling `setup.py docs` diff --git a/setup.cfg b/setup.cfg index c204ebab54..1536024195 100644 --- a/setup.cfg +++ b/setup.cfg @@ -4,7 +4,7 @@ [metadata] name = whylogs -version = 0.4.5-dev0 +version = 0.4.5-dev1 description = Profile and monitor your ML data pipeline end-to-end author = WhyLabs.ai author-email = support@whylabs.ai diff --git a/src/whylogs/_version.py b/src/whylogs/_version.py index bf17669e52..e541d45c51 100644 --- a/src/whylogs/_version.py +++ b/src/whylogs/_version.py @@ -1,3 +1,3 @@ """WhyLabs version number.""" -__version__ = "0.4.5-dev0" +__version__ = "0.4.5-dev1" From 48a7c617b81e9fe3a839a0fb6b6f7ce1e14ca1e8 Mon Sep 17 00:00:00 2001 From: "Leandro G. Almeida" Date: Fri, 9 Apr 2021 14:45:11 -0700 Subject: [PATCH 8/8] modify if logic --- src/whylogs/core/metrics/model_metrics.py | 37 ++++++++++++----------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/whylogs/core/metrics/model_metrics.py b/src/whylogs/core/metrics/model_metrics.py index e9506d913b..0a7a3f2677 100644 --- a/src/whylogs/core/metrics/model_metrics.py +++ b/src/whylogs/core/metrics/model_metrics.py @@ -20,7 +20,8 @@ def __init__(self, confusion_matrix: ConfusionMatrix = None, self.model_type = model_type if confusion_matrix is not None and regression_metrics is not None: - raise NotImplementedError("Regression Metrics together with Confusion Matrix not implemented yet") + raise NotImplementedError( + "Regression Metrics together with Confusion Matrix not implemented yet") if confusion_matrix is not None: if (self.model_type == ModelType.REGRESSION): @@ -38,14 +39,17 @@ def __init__(self, confusion_matrix: ConfusionMatrix = None, def to_protobuf(self, ) -> ModelMetricsMessage: return ModelMetricsMessage( scoreMatrix=self.confusion_matrix.to_protobuf() if self.confusion_matrix else None, - regressionMetrics=self.regression_metrics.to_protobuf() if self.regression_metrics else None, + regressionMetrics=self.regression_metrics.to_protobuf( + ) if self.regression_metrics else None, modelType=self.model_type) @classmethod def from_protobuf(cls, message, ): return ModelMetrics( - confusion_matrix=ConfusionMatrix.from_protobuf(message.scoreMatrix), - regression_metrics=RegressionMetrics.from_protobuf(message.regressionMetrics), + confusion_matrix=ConfusionMatrix.from_protobuf( + message.scoreMatrix), + regression_metrics=RegressionMetrics.from_protobuf( + message.regressionMetrics), model_type=message.modelType) def compute_confusion_matrix(self, predictions: List[Union[str, int, bool, float]], @@ -82,10 +86,12 @@ def compute_regression_metrics(self, predictions: List[Union[float, int]], targets: List[Union[float, int]], target_field: str = None, prediction_field: str = None): - regression_metrics = RegressionMetrics(target_field=target_field, prediction_field=prediction_field) + regression_metrics = RegressionMetrics( + target_field=target_field, prediction_field=prediction_field) regression_metrics.add(predictions, targets) if self.regression_metrics: - self.regression_metrics = self.regression_metrics.merge(regression_metrics) + self.regression_metrics = self.regression_metrics.merge( + regression_metrics) else: self.regression_metrics = regression_metrics @@ -97,20 +103,15 @@ def merge(self, other): if other is None: return self - model_type = ModelType.UNKNOWN - - if (self.model_type not in (ModelType.REGRESSION, ModelType.CLASSIFICATION)): - if other.model_type in (ModelType.REGRESSION, ModelType.CLASSIFICATION): - model_type = other.model_type - - elif other.model_type not in (ModelType.REGRESSION, ModelType.CLASSIFICATION): - if self.model_type in (ModelType.REGRESSION, ModelType.CLASSIFICATION): - model_type = self.model_type - else: - model_type = self.model_type + model_type = self.model_type + if model_type not in (ModelType.REGRESSION, ModelType.CLASSIFICATION): + model_type = other.model_type + if model_type not in (ModelType.REGRESSION, ModelType.CLASSIFICATION): + model_type = ModelType.UNKNOWN return ModelMetrics( - confusion_matrix=self.confusion_matrix.merge(other.confusion_matrix) if self.confusion_matrix else None, + confusion_matrix=self.confusion_matrix.merge( + other.confusion_matrix) if self.confusion_matrix else None, regression_metrics=self.regression_metrics.merge( other.regression_metrics)if self.regression_metrics else None, model_type=model_type)