Partially fix #412 by adding a dedicated method to evaluating y_max (#…

…415)

* Partially Fix #412 by adding a dedicated method to evaluating y_max

* Fix typos

* Rename `y_max()` to `_target_max()`

* Return `None` when there is no feasible point in the target space.

* Adapt the tests to the new TargetSpace.max behaviour · Issue #412

Now the TargetSpace.max returns None if there is no sampled feasible
point or no point was sampled at all.

bayes_opt/bayesian_optimization.py

@@ -95,7 +95,7 @@ class BayesianOptimization(Observable):
         If True, the optimizer will allow duplicate points to be registered.
         This behavior may be desired in high noise situations where repeatedly probing
         the same point will give different answers. In other situations, the acquisition
-        may occasionaly generate a duplicate point.
+        may occasionally generate a duplicate point.
 
     Methods
     -------
@@ -226,7 +226,7 @@ def suggest(self, utility_function):
         suggestion = acq_max(ac=utility_function.utility,
                              gp=self._gp,
                              constraint=self.constraint,
-                             y_max=self._space.target.max(),
+                             y_max=self._space._target_max(),
                              bounds=self._space.bounds,
                              random_state=self._random_state)
 
@@ -276,7 +276,7 @@ def maximize(self,
             An instance of bayes_opt.util.UtilityFunction.
             If nothing is passed, a default using ucb is used
 
-        All other parameters are unused, and are only available to ensure backwards compatability - these
+        All other parameters are unused, and are only available to ensure backwards compatibility - these
         will be removed in a future release
         """
         self._prime_subscriptions()

bayes_opt/logger.py

@@ -106,7 +106,7 @@ def _header(self, instance):
         return line + "\n" + ("-" * self._header_length)
 
     def _is_new_max(self, instance):
-        if instance.max["target"] is None:
+        if instance.max is None:
             # During constrained optimization, there might not be a maximum
             # value since the optimizer might've not encountered any points
             # that fulfill the constraints.

bayes_opt/observer.py

@@ -23,8 +23,12 @@ def __init__(self):
     def _update_tracker(self, event, instance):
         if event == Events.OPTIMIZATION_STEP:
             self._iterations += 1
+
+            if instance.max is None:
+                return
 
             current_max = instance.max
+
             if (self._previous_max is None
                     or current_max["target"] > self._previous_max):
                 self._previous_max = current_max["target"]

bayes_opt/target_space.py

@@ -214,7 +214,7 @@ def register(self, params, target, constraint_value=None):
 
     def probe(self, params):
         """
-        Evaulates a single point x, to obtain the value y and then records them
+        Evaluates a single point x, to obtain the value y and then records them
         as observations.
 
         Notes
@@ -265,44 +265,46 @@ def random_sample(self):
             data.T[col] = self.random_state.uniform(lower, upper, size=1)
         return data.ravel()
 
+    def _target_max(self):
+        """Get maximum target value found.
+        
+        If there is a constraint present, the maximum value that fulfills the
+        constraint is returned."""
+        if len(self.target) == 0:
+            return None
+
+        if self._constraint is None:
+            return self.target.max()
+
+        allowed = self._constraint.allowed(self._constraint_values)
+        if allowed.any():
+            return self.target[allowed].max()
+
+        return None
+
     def max(self):
         """Get maximum target value found and corresponding parameters.
         
         If there is a constraint present, the maximum value that fulfills the
         constraint is returned."""
-        if self._constraint is None:
-            try:
-                res = {
-                    'target': self.target.max(),
-                    'params': dict(
-                        zip(self.keys, self.params[self.target.argmax()])
-                    )
-                }
-            except ValueError:
-                res = {}
-            return res
-        else:
-            allowed = self._constraint.allowed(self._constraint_values)
-            if allowed.any():
-                # Getting of all points that fulfill the constraints, find the
-                # one with the maximum value for the target function.
-                sorted = np.argsort(self.target)
-                idx = sorted[allowed[sorted]][-1]
-                # there must be a better way to do this, right?
-                res = {
-                    'target': self.target[idx],
-                    'params': dict(
-                        zip(self.keys, self.params[idx])
-                    ),
-                    'constraint': self._constraint_values[idx]
-                }
-            else:
-                res = {
-                    'target': None,
-                    'params': None,
-                    'constraint': None
-                }
-            return res
+        target_max = self._target_max()
+
+        if target_max is None:
+            return None
+
+        target_max_idx = np.where(self.target == target_max)[0][0]
+
+        res = {
+                'target': target_max,
+                'params': dict(
+                zip(self.keys, self.params[target_max_idx])
+            )
+        }
+
+        if self._constraint is not None:
+            res['constraint'] = self._constraint_values[target_max_idx]
+
+        return res
 
     def res(self):
         """Get all target values and constraint fulfillment for all parameters.

tests/test_target_space.py

@@ -163,16 +163,46 @@ def test_random_sample():
         assert all(random_sample <= space.bounds[:, 1])
 
 
+def test_y_max():
+    space = TargetSpace(target_func, PBOUNDS)
+    assert space._target_max() == None
+    space.probe(params={"p1": 1, "p2": 2})
+    space.probe(params={"p1": 5, "p2": 1})
+    space.probe(params={"p1": 0, "p2": 1})
+    assert space._target_max() == 6
+
+def test_y_max_with_constraint():
+    constraint = ConstraintModel(lambda p1, p2: p1-p2, -2, 2)
+    space = TargetSpace(target_func, PBOUNDS, constraint)
+    assert space._target_max() == None
+    space.probe(params={"p1": 1, "p2": 2}) # Feasible
+    space.probe(params={"p1": 5, "p2": 1}) # Unfeasible
+    space.probe(params={"p1": 0, "p2": 1}) # Feasible
+    assert space._target_max() == 3
+
+
+
 def test_max():
     space = TargetSpace(target_func, PBOUNDS)
 
-    assert space.max() == {}
+    assert space.max() == None
     space.probe(params={"p1": 1, "p2": 2})
     space.probe(params={"p1": 5, "p2": 4})
     space.probe(params={"p1": 2, "p2": 3})
     space.probe(params={"p1": 1, "p2": 6})
     assert space.max() == {"params": {"p1": 5, "p2": 4}, "target": 9}
 
+def test_max_with_constraint():
+    constraint = ConstraintModel(lambda p1, p2: p1-p2, -2, 2)
+    space = TargetSpace(target_func, PBOUNDS, constraint=constraint)
+
+    assert space.max() == None
+    space.probe(params={"p1": 1, "p2": 2}) # Feasible
+    space.probe(params={"p1": 5, "p2": 8}) # Unfeasible
+    space.probe(params={"p1": 2, "p2": 3}) # Feasible
+    space.probe(params={"p1": 1, "p2": 6}) # Unfeasible
+    assert space.max() == {"params": {"p1": 2, "p2": 3}, "target": 5, "constraint": -1}
+
 
 def test_res():
     space = TargetSpace(target_func, PBOUNDS)