From 3f757b8ba19a04f76ad3f1b4ad3115d4a4735a79 Mon Sep 17 00:00:00 2001 From: Vadym Doroshenko <53558779+dvadym@users.noreply.github.com> Date: Tue, 18 Jul 2023 16:10:42 +0200 Subject: [PATCH] Make histogram.max_value to be method instead of property (#471) --- analysis/parameter_tuning.py | 4 ++-- analysis/tests/parameter_tuning_test.py | 18 +++++++++++------- pipeline_dp/dataset_histograms/histograms.py | 5 ++--- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/analysis/parameter_tuning.py b/analysis/parameter_tuning.py index a72ed190..a0ff695a 100644 --- a/analysis/parameter_tuning.py +++ b/analysis/parameter_tuning.py @@ -196,7 +196,7 @@ def _find_candidates_quantiles(histogram: histograms.Histogram, """Implementation of QUANTILES strategy.""" quantiles_to_use = [0.9, 0.95, 0.98, 0.99, 0.995] candidates = histogram.quantiles(quantiles_to_use) - candidates.append(histogram.max_value) + candidates.append(histogram.max_value()) candidates = list(set(candidates)) # remove duplicates candidates.sort() return candidates[:max_candidates] @@ -205,7 +205,7 @@ def _find_candidates_quantiles(histogram: histograms.Histogram, def _find_candidates_constant_relative_step(histogram: histograms.Histogram, max_candidates: int) -> List[int]: """Implementation of CONSTANT_RELATIVE_STEP strategy.""" - max_value = histogram.max_value + max_value = histogram.max_value() assert max_value >= 1, "max_value has to be >= 1." max_candidates = min(max_candidates, max_value) assert max_candidates > 0, "max_candidates have to be positive" diff --git a/analysis/tests/parameter_tuning_test.py b/analysis/tests/parameter_tuning_test.py index a4475c1e..8762c64f 100644 --- a/analysis/tests/parameter_tuning_test.py +++ b/analysis/tests/parameter_tuning_test.py @@ -63,9 +63,10 @@ def test_find_candidate_parameters_quantiles_strategy( ): mock_l0_histogram = histograms.Histogram(None, None) mock_l0_histogram.quantiles = mock.Mock(return_value=[1, 1, 2]) - setattr(mock_l0_histogram.__class__, 'max_value', 6) + mock_l0_histogram.max_value = mock.Mock(return_value=6) mock_linf_histogram = histograms.Histogram(None, None) mock_linf_histogram.quantiles = mock.Mock(return_value=[3, 6, 6]) + mock_linf_histogram.max_value = mock.Mock(return_value=6) mock_histograms = histograms.DatasetHistograms(mock_l0_histogram, None, mock_linf_histogram, @@ -90,9 +91,10 @@ def test_find_candidate_parameters_maximum_number_of_candidates_is_respected_whe self): mock_l0_histogram = histograms.Histogram(None, None) mock_l0_histogram.quantiles = mock.Mock(return_value=[1, 2, 3]) - setattr(mock_l0_histogram.__class__, 'max_value', 6) + mock_l0_histogram.max_value = mock.Mock(return_value=6) mock_linf_histogram = histograms.Histogram(None, None) mock_linf_histogram.quantiles = mock.Mock(return_value=[4, 5, 6]) + mock_linf_histogram.max_value = mock.Mock(return_value=6) mock_histograms = histograms.DatasetHistograms(mock_l0_histogram, None, mock_linf_histogram, @@ -115,9 +117,10 @@ def test_find_candidate_parameters_more_candidates_for_l_0_when_not_so_many_l_in self): mock_l0_histogram = histograms.Histogram(None, None) mock_l0_histogram.quantiles = mock.Mock(return_value=[1, 2, 3, 4, 5]) - setattr(mock_l0_histogram.__class__, 'max_value', 6) + mock_l0_histogram.max_value = mock.Mock(return_value=6) mock_linf_histogram = histograms.Histogram(None, None) mock_linf_histogram.quantiles = mock.Mock(return_value=[6, 7]) + mock_linf_histogram.max_value = mock.Mock(return_value=6) mock_histograms = histograms.DatasetHistograms(mock_l0_histogram, None, mock_linf_histogram, @@ -145,9 +148,10 @@ def test_find_candidate_parameters_more_candidates_for_l_inf_when_not_so_many_l_ self): mock_l0_histogram = histograms.Histogram(None, None) mock_l0_histogram.quantiles = mock.Mock(return_value=[1]) - setattr(mock_l0_histogram.__class__, 'max_value', 8) + mock_l0_histogram.max_value = mock.Mock(return_value=8) mock_linf_histogram = histograms.Histogram(None, None) mock_linf_histogram.quantiles = mock.Mock(return_value=[3, 4, 5, 6, 7]) + mock_linf_histogram.max_value = mock.Mock(return_value=8) mock_histograms = histograms.DatasetHistograms(mock_l0_histogram, None, mock_linf_histogram, @@ -204,7 +208,7 @@ def test_find_candidate_parameters_more_candidates_for_l_inf_when_not_so_many_l_ def test_find_candidate_parameters_constant_relative_ste_strategy( self, max_value, max_candidates, expected_candidates): mock_l0_histogram = histograms.Histogram(None, None) - setattr(histograms.Histogram, 'max_value', max_value) + mock_l0_histogram.max_value = mock.Mock(return_value=max_value) mock_histograms = histograms.DatasetHistograms(mock_l0_histogram, None, None, None, None) @@ -245,7 +249,7 @@ def test_tune_count(self): # Assert. tune_result, per_partition_utility_analysis = result per_partition_utility_analysis = list(per_partition_utility_analysis) - self.assertLen(per_partition_utility_analysis, 40) + self.assertLen(per_partition_utility_analysis, 10) tune_result = list(tune_result)[0] @@ -253,7 +257,7 @@ def test_tune_count(self): self.assertEqual(contribution_histograms, tune_result.contribution_histograms) utility_reports = tune_result.utility_reports - self.assertLen(utility_reports, 4) + self.assertLen(utility_reports, 1) self.assertIsInstance(utility_reports[0], metrics.UtilityReport) self.assertLen(utility_reports[0].metric_errors, 1) self.assertEqual(utility_reports[0].metric_errors[0].metric, diff --git a/pipeline_dp/dataset_histograms/histograms.py b/pipeline_dp/dataset_histograms/histograms.py index 683f4b61..d0682e8c 100644 --- a/pipeline_dp/dataset_histograms/histograms.py +++ b/pipeline_dp/dataset_histograms/histograms.py @@ -75,7 +75,6 @@ def total_count(self): def total_sum(self): return sum([bin.sum for bin in self.bins]) - @property def max_value(self): return self.bins[-1].max @@ -141,9 +140,9 @@ def compute_ratio_dropped( ratio_dropped = [] bins = contribution_histogram.bins previous_value = bins[-1].lower # lower of the largest bin. - if contribution_histogram.max_value != previous_value: + if contribution_histogram.max_value() != previous_value: # Add ratio for max_value when max_value is not lower in bins. - ratio_dropped.append((contribution_histogram.max_value, 0.0)) + ratio_dropped.append((contribution_histogram.max_value(), 0.0)) for bin in bins[::-1]: current_value = bin.lower