Skip to content

Commit

Permalink
Merge branch 'main' into update_parameter_tuning
Browse files Browse the repository at this point in the history
  • Loading branch information
dvadym committed Jul 18, 2023
2 parents c504991 + 3f757b8 commit cb278ef
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
4 changes: 2 additions & 2 deletions analysis/parameter_tuning.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,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]
Expand All @@ -211,7 +211,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"
Expand Down
18 changes: 11 additions & 7 deletions analysis/tests/parameter_tuning_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,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,
Expand All @@ -91,9 +92,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,
Expand All @@ -116,9 +118,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,
Expand Down Expand Up @@ -146,9 +149,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,
Expand Down Expand Up @@ -205,7 +209,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)
Expand Down Expand Up @@ -246,15 +250,15 @@ 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]

self.assertEqual(tune_options, tune_result.options)
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,
Expand Down
5 changes: 2 additions & 3 deletions pipeline_dp/dataset_histograms/histograms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit cb278ef

Please sign in to comment.