Skip to content

Commit

Permalink
Followup fix to MOO refactor
Browse files Browse the repository at this point in the history
Summary:
In D28869199 (facebook@595095e), I changed the multi objective data model, so that instead of relying on objective.metric.lower_is_better to indicate directionality, we can instead use objective.minimize.

I forgot to update the validation we have that errors if the directionality of an objective threshold doesn't match the directionality of an objective. The validation is fine, but the way we determine the directionality of the objective has to change (i.e. check minimize now instead of lower_is_better).

Reviewed By: Balandat

Differential Revision: D29165965

fbshipit-source-id: 4b935f2a018da2666d487e21ae7b659559e42a3f
  • Loading branch information
ldworkin authored and facebook-github-bot committed Jun 17, 2021
1 parent e554b43 commit 1aa2c2c
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 36 deletions.
54 changes: 26 additions & 28 deletions ax/core/optimization_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,25 +364,9 @@ def _validate_optimization_config(
outcome_constraints = outcome_constraints or []
objective_thresholds = objective_thresholds or []

# Verify we aren't optimizing too many objectives.
objective_metrics_by_name = {
metric.name: metric for metric in objective.metrics
}
# Warn if thresholds on objective_metrics bound from the wrong direction.
for threshold in objective_thresholds:
metric_name = threshold.metric.name
if metric_name in objective_metrics_by_name:
lower_is_better = threshold.metric.lower_is_better
bounded_above = threshold.op == ComparisonOp.LEQ
is_aligned = lower_is_better == bounded_above
if not (is_aligned or lower_is_better is None):
raise ValueError(
make_wrong_direction_warning(
metric_name=metric_name,
bounded_above=bounded_above,
lower_is_better=lower_is_better,
)
)
maybe_raise_wrong_direction_warning(
objective=objective, objective_thresholds=objective_thresholds
)

unconstrainable_metrics = objective.get_unconstrainable_metrics()
OptimizationConfig._validate_outcome_constraints(
Expand All @@ -402,12 +386,26 @@ def __repr__(self) -> str:
)


def make_wrong_direction_warning(
metric_name: str, bounded_above: bool, lower_is_better: Optional[bool]
) -> str:
return (
f"Constraint on {metric_name} bounds from "
f"{'above' if bounded_above else 'below'} "
f"but {metric_name} is being "
f"{'minimized' if lower_is_better else 'maximized'}."
).format(metric_name)
def maybe_raise_wrong_direction_warning(
objective: Objective,
objective_thresholds: List[ObjectiveThreshold],
) -> None:
"""Warn if thresholds on objective_metrics bound from the wrong direction."""
if not isinstance(objective, MultiObjective):
return

objectives_by_name = {obj.metric.name: obj for obj in objective.objectives}
for threshold in objective_thresholds:
metric_name = threshold.metric.name
if metric_name in objectives_by_name:
minimize = objectives_by_name[metric_name].minimize
bounded_above = threshold.op == ComparisonOp.LEQ
is_aligned = minimize == bounded_above
if not is_aligned:
message = (
f"Constraint on {metric_name} bounds from "
f"{'above' if bounded_above else 'below'} "
f"but {metric_name} is being "
f"{'minimized' if minimize else 'maximized'}."
).format(metric_name)
raise ValueError(message)
4 changes: 2 additions & 2 deletions ax/core/tests/test_optimization_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
MOOC_STR = (
"OptimizationConfig(objective=MultiObjective(objectives="
'[Objective(metric_name="m1", minimize=True), '
'Objective(metric_name="m2", minimize=True)]), '
'Objective(metric_name="m2", minimize=False)]), '
"outcome_constraints=[OutcomeConstraint(m2 >= -0.25%), "
"OutcomeConstraint(m2 <= 0.25%)], objective_thresholds=[])"
)
Expand Down Expand Up @@ -189,7 +189,7 @@ def setUp(self):
}
self.objectives = {
"o1": Objective(metric=self.metrics["m1"]),
"o2": Objective(metric=self.metrics["m2"], minimize=True),
"o2": Objective(metric=self.metrics["m2"], minimize=False),
"o3": Objective(metric=self.metrics["m3"], minimize=False),
}
self.objective = Objective(metric=self.metrics["m1"], minimize=False)
Expand Down
13 changes: 8 additions & 5 deletions ax/modelbridge/tests/test_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,12 @@ def test_MOO_PAREGO(self):
def test_MOO_EHVI(self):
single_obj_exp = get_branin_experiment(with_batch=True)
metrics = single_obj_exp.optimization_config.objective.metrics
metrics[0].lower_is_better = True
objective_thresholds = [
ObjectiveThreshold(
metric=metrics[0], bound=0.0, relative=False, op=ComparisonOp.GEQ
)
]
# ValueError: Multi-objective optimization requires multiple objectives.
with self.assertRaises(ValueError):
get_MOO_EHVI(
experiment=single_obj_exp,
Expand All @@ -280,12 +280,15 @@ def test_MOO_EHVI(self):
)
multi_obj_exp = get_branin_experiment_with_multi_objective(with_batch=True)
metrics = multi_obj_exp.optimization_config.objective.metrics
metrics[0].lower_is_better = False
metrics[1].lower_is_better = True
multi_objective_thresholds = [
ObjectiveThreshold(metric=metrics[0], bound=0.0, relative=False),
ObjectiveThreshold(metric=metrics[1], bound=0.0, relative=False),
ObjectiveThreshold(
metric=metrics[0], bound=0.0, relative=False, op=ComparisonOp.GEQ
),
ObjectiveThreshold(
metric=metrics[1], bound=0.0, relative=False, op=ComparisonOp.GEQ
),
]
# ValueError: MultiObjectiveOptimization requires non-empty data.
with self.assertRaises(ValueError):
get_MOO_EHVI(
experiment=multi_obj_exp,
Expand Down
1 change: 0 additions & 1 deletion ax/utils/testing/core_stubs.py
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,6 @@ def get_branin_multi_objective() -> Objective:
Objective(metric=get_branin_metric(name="branin_a")),
Objective(metric=get_branin_metric(name="branin_b")),
],
minimize=False,
)


Expand Down

0 comments on commit 1aa2c2c

Please sign in to comment.