From 755b1f70401fc83a3915620235afc9cacf72d88a Mon Sep 17 00:00:00 2001 From: Richard Zhang Date: Wed, 28 Feb 2024 14:09:19 -0800 Subject: [PATCH] Add clipping boolean flag. PiperOrigin-RevId: 611223860 --- .../experimenters/shifting_experimenter.py | 14 +++--- .../shifting_experimenter_test.py | 46 ++++++++++--------- vizier/pyvizier/converters/core.py | 24 +++++----- vizier/pyvizier/converters/core_test.py | 27 ++++++++++- 4 files changed, 72 insertions(+), 39 deletions(-) diff --git a/vizier/_src/benchmarks/experimenters/shifting_experimenter.py b/vizier/_src/benchmarks/experimenters/shifting_experimenter.py index b8093c8e3..ab9e401a6 100644 --- a/vizier/_src/benchmarks/experimenters/shifting_experimenter.py +++ b/vizier/_src/benchmarks/experimenters/shifting_experimenter.py @@ -69,7 +69,10 @@ def __init__( # Converter should be in the underlying extpr space. self._converter = converters.TrialToArrayConverter.from_study_config( - study_config=exptr_problem_statement, scale=False) + study_config=exptr_problem_statement, + scale=False, + should_clip=should_restrict, + ) self._problem_statement = copy.deepcopy(exptr_problem_statement) if should_restrict: @@ -88,10 +91,9 @@ def __init__( ) # Shift the bounds to maintain valid bounds. if shift >= 0: - new_bounds = (bounds[0], bounds[1] - shift) + new_bounds = (bounds[0] + shift, bounds[1]) else: - # Shift is negative so this restricts the bounds. - new_bounds = (bounds[0] - shift, bounds[1]) + new_bounds = (bounds[0], bounds[1] + shift) self._problem_statement.search_space.add( pyvizier.ParameterConfig.factory( name=parameter.name, @@ -106,7 +108,7 @@ def problem_statement(self) -> pyvizier.ProblemStatement: return copy.deepcopy(self._problem_statement) def evaluate(self, suggestions: Sequence[pyvizier.Trial]) -> None: - """Evaluate the trials after shifting their parameters by +shift.""" + """Evaluate the trials after shifting their parameters by -shift.""" previous_parameters = [suggestion.parameters for suggestion in suggestions] self._offset(suggestions, self._shift) self._exptr.evaluate(suggestions) @@ -119,7 +121,7 @@ def _offset(self, suggestions: Sequence[pyvizier.Trial], """Offsets parameter values (OOB values are clipped).""" for suggestion in suggestions: features = self._converter.to_features([suggestion]) - new_parameters = self._converter.to_parameters(features + shift)[0] + new_parameters = self._converter.to_parameters(features - shift)[0] suggestion.parameters = new_parameters def __repr__(self): diff --git a/vizier/_src/benchmarks/experimenters/shifting_experimenter_test.py b/vizier/_src/benchmarks/experimenters/shifting_experimenter_test.py index bcc665292..daa3670cd 100644 --- a/vizier/_src/benchmarks/experimenters/shifting_experimenter_test.py +++ b/vizier/_src/benchmarks/experimenters/shifting_experimenter_test.py @@ -56,10 +56,12 @@ def test_numpy_experimenter(self, func): t = pyvizier.Trial(parameters={ param.name: float(index) for index, param in enumerate(parameters) }) - t_shifted = pyvizier.Trial(parameters={ - param.name: float(index) + shift - for index, param in enumerate(parameters) - }) + t_shifted = pyvizier.Trial( + parameters={ + param.name: float(index) - shift + for index, param in enumerate(parameters) + } + ) exptr.evaluate([t_shifted]) shifted_exptr.evaluate([t]) @@ -74,8 +76,8 @@ def test_numpy_experimenter(self, func): shifted_parameters = shifted_exptr.problem_statement( ).search_space.parameters for param, shifted_param in zip(parameters, shifted_parameters): - self.assertEqual(param.bounds[0], shifted_param.bounds[0]) - self.assertEqual(param.bounds[1] - shift, shifted_param.bounds[1]) + self.assertEqual(param.bounds[0] + shift, shifted_param.bounds[0]) + self.assertEqual(param.bounds[1], shifted_param.bounds[1]) def test_evaluate_shift(self): dim = 2 @@ -94,9 +96,10 @@ def test_evaluate_shift(self): }) t_shifted = pyvizier.Trial( parameters={ - param.name: float(index) + shift[index] + param.name: float(index) - shift[index] for index, param in enumerate(parameters) - }) + } + ) exptr.evaluate([t_shifted]) shifted_exptr.evaluate([t]) @@ -119,13 +122,12 @@ def test_shift_backward(self): # Test shift within bounds trial = pyvizier.Trial(parameters={'x0': 3.0, 'x1': 1.0}) shifted_exptr._offset([trial], shift=-shift) - self.assertEqual(trial.parameters.as_dict(), { - 'x0': 3.0 - 1.2, - 'x1': 1.0 + 2.3 - }) + self.assertEqual( + trial.parameters.as_dict(), {'x0': 3.0 + 1.2, 'x1': 1.0 - 2.3} + ) # Test shift in out of bounds trial = pyvizier.Trial(parameters={'x0': -5.0, 'x1': 5.0}) - shifted_exptr._offset([trial], shift=-shift) + shifted_exptr._offset([trial], shift=shift) self.assertEqual(trial.parameters.as_dict(), {'x0': -5.0, 'x1': 5.0}) def test_shift_forward(self): @@ -139,19 +141,18 @@ def test_shift_forward(self): # Test shift within bounds trial = pyvizier.Trial(parameters={'x0': 3.0, 'x1': 1.0}) shifted_exptr._offset([trial], shift=shift) - self.assertEqual(trial.parameters.as_dict(), { - 'x0': 3.0 + 1.2, - 'x1': 1.0 - 2.3 - }) + self.assertEqual( + trial.parameters.as_dict(), {'x0': 3.0 - 1.2, 'x1': 1.0 + 2.3} + ) # Test shift in out of bounds trial = pyvizier.Trial(parameters={'x0': 5.0, 'x1': -5.0}) - shifted_exptr._offset([trial], shift=shift) + shifted_exptr._offset([trial], shift=-shift) self.assertEqual(trial.parameters.as_dict(), {'x0': 5.0, 'x1': -5.0}) @parameterized.parameters((True,), (False,)) def test_shift_forward_oob(self, should_restrict): dim = 2 - shift = np.array([2.2, -2.3]) + shift = np.array([-2.2, 2.3]) func = bbob.Sphere exptr = numpy_experimenter.NumpyExperimenter( func, bbob.DefaultBBOBProblemStatement(dim) @@ -166,8 +167,11 @@ def test_shift_forward_oob(self, should_restrict): # Test OOB shifts stay within bounds. shifted_exptr._offset([trial], shift=shift) - # x0 is shifted OOB, so clip at 5.0. - self.assertEqual(trial.parameters.as_dict(), {'x0': 5.0, 'x1': 1.0 - 2.3}) + # x0 is shifted OOB, so clip at 5.0 if should_restrict=True. + if should_restrict: + self.assertEqual(trial.parameters.as_dict(), {'x0': 5.0, 'x1': 1.0 - 2.3}) + else: + self.assertEqual(trial.parameters.as_dict(), {'x0': 5.2, 'x1': 1.0 - 2.3}) def test_large_shift(self): dim = 2 diff --git a/vizier/pyvizier/converters/core.py b/vizier/pyvizier/converters/core.py index 541d80c1f..925d3436e 100644 --- a/vizier/pyvizier/converters/core.py +++ b/vizier/pyvizier/converters/core.py @@ -556,6 +556,7 @@ def __init__( onehot_embed: bool = False, converts_to_parameter: bool = True, pad_oovs: bool = True, + should_clip: bool = True, ): """Init. @@ -577,6 +578,7 @@ def __init__( returns None pad_oovs: If True, pad the out-of-vocabulary dimensions to onehot embedding. + should_clip: If True, clipping should be applied to parameter values. """ self._converts_to_parameter = converts_to_parameter self._parameter_config = copy.deepcopy(parameter_config) @@ -614,6 +616,7 @@ def __init__( spec = self.onehot_encoder.output_spec self._output_spec = spec + self._should_clip = should_clip def convert(self, trials: Sequence[pyvizier.TrialSuggestion]) -> np.ndarray: """Returns an array of shape [len(trials), output_spec.num_dimensions]. @@ -665,15 +668,13 @@ def _to_parameter_value( return None elif self.parameter_config.type == pyvizier.ParameterType.DOUBLE: # Input parameter was DOUBLE. Output is also DOUBLE. - return pyvizier.ParameterValue( - float( - np.clip( - value, - self._parameter_config.bounds[0], - self._parameter_config.bounds[1], - ) - ) - ) + if self._should_clip: + value = np.clip( + value, + self._parameter_config.bounds[0], + self._parameter_config.bounds[1], + ) + return pyvizier.ParameterValue(float(value)) elif self.output_spec.type == NumpyArraySpecType.CONTINUOUS: # The parameter config is originally discrete, but continuified. # Round to the closest number. @@ -1050,7 +1051,6 @@ def to_parameters( ) -> List[pyvizier.ParameterDict]: """Convert to nearest feasible parameter value. NaNs trigger errors.""" # TODO: NaNs should be ignored instead of triggering errors. - # TODO: Add a boolean flag to disable automatic clipping. # Validate features's shape, and create empty ParameterDicts. parameters = [ @@ -1245,7 +1245,6 @@ def to_xy(self, trials) -> Tuple[np.ndarray, np.ndarray]: def to_parameters(self, arr: np.ndarray) -> Sequence[pyvizier.ParameterDict]: """Convert to nearest feasible parameter value. NaNs are preserved.""" - # TODO: Add a boolean flag to disable automatic clipping. arrformat = DictOf2DArrays(self._impl.to_features([])) return self._impl.to_parameters(arrformat.dict_like(arr)) @@ -1258,6 +1257,7 @@ def from_study_config( pad_oovs: bool = True, max_discrete_indices: int = 0, flip_sign_for_minimization_metrics: bool = True, + should_clip=True, dtype=np.float64, ) -> 'TrialToArrayConverter': """From study config. @@ -1273,6 +1273,7 @@ def from_study_config( default is different from the default in DefaultModelInputConverter. flip_sign_for_minimization_metrics: If True, flips the metric signs so that every metric maximizes. + should_clip: Whether or not clipping should be done. dtype: dtype Returns: @@ -1287,6 +1288,7 @@ def create_input_converter(parameter): onehot_embed=True, float_dtype=dtype, pad_oovs=pad_oovs, + should_clip=should_clip, ) def create_output_converter(metric): diff --git a/vizier/pyvizier/converters/core_test.py b/vizier/pyvizier/converters/core_test.py index a9805d6e0..53f5c9426 100644 --- a/vizier/pyvizier/converters/core_test.py +++ b/vizier/pyvizier/converters/core_test.py @@ -893,7 +893,7 @@ def test_double_into_double_log_inverse(self, dtype): float_dtype=dtype, ) - scaled = np.asarray([[0.0], [0.5], [1.0]], dtype) + scaled = np.asarray([[0.0], [0.5], [1.1]], dtype) # Pytype still thinks `actual` entries might be None, hence we specify type. actual: list[pyvizier.ParameterValue] = converter.to_parameter_values( # pytype:disable=annotation-type-mismatch scaled @@ -904,6 +904,31 @@ def test_double_into_double_log_inverse(self, dtype): self.assertAlmostEqual(actual[2].value, 100.0, delta=1e-2) self.assertLessEqual(actual[2].value, 100.0) + @parameterized.parameters([ + dict(dtype=np.float32), + dict(dtype=np.float64), + dict(dtype='float32'), + dict(dtype='float64'), + ]) + def test_double_into_double_log_inverse_noclipping(self, dtype): + converter = core.DefaultModelInputConverter( + pyvizier.ParameterConfig.factory( + 'x1', bounds=(1e-4, 1e2), scale_type=pyvizier.ScaleType.LOG + ), + scale=True, + float_dtype=dtype, + should_clip=False, + ) + + scaled = np.asarray([[0.0], [0.5], [1.1]], dtype) + # Pytype still thinks `actual` entries might be None, hence we specify type. + actual: list[pyvizier.ParameterValue] = converter.to_parameter_values( # pytype:disable=annotation-type-mismatch + scaled + ) + self.assertAlmostEqual(actual[0].value, 1e-4, delta=1e-6) + self.assertAlmostEqual(actual[1].value, 0.1, delta=1e-5) + self.assertAlmostEqual(actual[2].value, 398, delta=1) + @parameterized.parameters([ dict(dtype=np.float32), dict(dtype=np.float64),