From e10543896ca5201406272b3f168b3c824a27a9b7 Mon Sep 17 00:00:00 2001 From: Daniel Cohen Date: Wed, 4 Sep 2024 14:28:19 -0700 Subject: [PATCH] make debugging easier by saying which constraint is bad (#2737) Summary: Pull Request resolved: https://github.com/facebook/Ax/pull/2737 Would have made debugging this problem way easier https://fb.workplace.com/groups/ae.feedback/posts/8591041154263824/?comment_id=8591648994203040&reply_comment_id=8591748957526377 Generally if there are a bunch of constraints, it's helpful to let the user know which constraint we have a problem with. Reviewed By: eonofrey Differential Revision: D62154912 fbshipit-source-id: c6cb6bbfdfb77192aaa46999794a105efb8c94e8 --- ax/service/tests/test_instantiation_utils.py | 3 +++ ax/service/utils/instantiation.py | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ax/service/tests/test_instantiation_utils.py b/ax/service/tests/test_instantiation_utils.py index aa9c9d1105c..bfa75db3f79 100644 --- a/ax/service/tests/test_instantiation_utils.py +++ b/ax/service/tests/test_instantiation_utils.py @@ -70,6 +70,9 @@ def test_constraint_from_str(self) -> None: }, ) + with self.assertRaisesRegex(AssertionError, "Outcome constraint 'm1"): + InstantiationBase.outcome_constraint_from_str("m1 == 2*m2") + self.assertEqual(three_val_constaint.bound, 3.0) with self.assertRaisesRegex(ValueError, "Parameter constraint should"): InstantiationBase.constraint_from_str( diff --git a/ax/service/utils/instantiation.py b/ax/service/utils/instantiation.py index ba5965a64df..afcbcbd292a 100644 --- a/ax/service/utils/instantiation.py +++ b/ax/service/utils/instantiation.py @@ -477,7 +477,8 @@ def outcome_constraint_from_str( """Parse string representation of an outcome constraint.""" tokens = representation.split() assert len(tokens) == 3 and tokens[1] in COMPARISON_OPS, ( - "Outcome constraint should be of form `metric_name >= x`, where x is a " + f"Outcome constraint '{representation}' should be of " + "form `metric_name >= x`, where x is a " "float bound and comparison operator is >= or <=." ) op = COMPARISON_OPS[tokens[1]] @@ -489,7 +490,9 @@ def outcome_constraint_from_str( bound_repr = bound_repr[:-1] bound = float(bound_repr) except ValueError: - raise ValueError("Outcome constraint bound should be a float.") + raise ValueError( + f"Outcome constraint bound should be a float for '{representation}'." + ) return OutcomeConstraint( cls._make_metric( name=tokens[0],