From 512f01b2da36f5a001d63b2556adc83bc881f1cf Mon Sep 17 00:00:00 2001 From: Shruti Patel Date: Wed, 8 Apr 2026 14:00:58 -0700 Subject: [PATCH] Improve validate_applicable_state error messages - Medium Priority Updates (#5127) Summary: Improve validate_applicable_state error messages across ax/analysis to be more user-friendly and descriptive: - Remove self-referential analysis class names from error messages (e.g., "GenerationStrategyGraph requires...", "PredictableMetricsAnalysis requires...", "BaselineImprovementAnalysis requires...") and replace them with action-oriented phrasing (e.g., "A GenerationStrategy must be provided to visualize..."). - Add parenthetical explanations alongside retained domain class names (OptimizationConfig, OutcomeConstraint, ChoiceParameter) to clarify their meaning for users unfamiliar with Ax internals. - Reword the base "Requires an Experiment." message to the more descriptive "An Experiment must be provided to compute this analysis." - Remove backtick formatting around class names in error strings. - Add purpose/context to error messages (e.g., "...to compute probability of feasibility", "...to evaluate baseline improvement"). - Replace "Objective" jargon with plain "single-objective optimization" language in OutcomeConstraintsAnalysis. - Update test assertions to match the new message wording. Audited in https://docs.google.com/document/d/1mxkLvXz0SvdWqO9UrVq5hsFY6MlcwDSk7oVRzdU6kN0/edit?tab=t.0 with the help of an AI Agent Reviewed By: bernardbeckerman Differential Revision: D97528390 --- .../graphviz/generation_strategy_graph.py | 10 +++- .../tests/test_generation_strategy_graph.py | 18 +++++- .../healthcheck/baseline_improvement.py | 5 +- .../healthcheck/predictable_metrics.py | 10 +++- .../tests/test_predictable_metrics.py | 2 +- ax/analysis/insights.py | 20 +++++-- ax/analysis/plotly/marginal_effects.py | 5 +- ax/analysis/plotly/p_feasible.py | 5 +- .../plotly/tests/test_bandit_rollout.py | 2 +- .../tests/test_constraint_feasibility.py | 2 +- .../plotly/tests/test_marginal_effects.py | 2 +- .../test_objective_p_feasible_frontier.py | 2 +- ax/analysis/plotly/tests/test_p_feasible.py | 15 ++++- .../plotly/tests/test_parallel_coordinates.py | 2 +- ax/analysis/plotly/tests/test_progression.py | 2 +- ax/analysis/plotly/tests/test_top_surfaces.py | 2 +- ax/analysis/tests/test_metric_summary.py | 2 +- ax/analysis/tests/test_overview.py | 55 ++++++++++++++++++- ax/analysis/tests/test_results.py | 2 +- ax/analysis/tests/test_utils.py | 12 ++++ ax/analysis/utils.py | 12 ++-- 21 files changed, 155 insertions(+), 32 deletions(-) diff --git a/ax/analysis/graphviz/generation_strategy_graph.py b/ax/analysis/graphviz/generation_strategy_graph.py index 89e1026ed74..31d12dab054 100644 --- a/ax/analysis/graphviz/generation_strategy_graph.py +++ b/ax/analysis/graphviz/generation_strategy_graph.py @@ -86,10 +86,16 @@ def validate_applicable_state( GenerationStrategyGraph requires a GenerationStrategy to be provided. """ if generation_strategy is None: - return "GenerationStrategyGraph requires a GenerationStrategy" + return ( + "A GenerationStrategy must be provided to visualize the " + "experiment's optimization workflow." + ) if len(generation_strategy._nodes) == 0: - return "GenerationStrategy has no nodes to visualize" + return ( + "The GenerationStrategy has no nodes to visualize. Ensure the " + "generation strategy is fully configured." + ) return None diff --git a/ax/analysis/graphviz/tests/test_generation_strategy_graph.py b/ax/analysis/graphviz/tests/test_generation_strategy_graph.py index a030bacfffe..3871dea6958 100644 --- a/ax/analysis/graphviz/tests/test_generation_strategy_graph.py +++ b/ax/analysis/graphviz/tests/test_generation_strategy_graph.py @@ -104,7 +104,23 @@ def test_validate_applicable_state_no_gs(self) -> None: generation_strategy=None, ) self.assertIsNotNone(result) - self.assertIn("requires a GenerationStrategy", result) + self.assertIn("GenerationStrategy must be provided", result) + + def test_validate_applicable_state_empty_nodes(self) -> None: + """Test that validation fails when GenerationStrategy has no nodes.""" + analysis = GenerationStrategyGraph() + gs = GenerationStrategy( + steps=[ + GenerationStep(generator=Generators.SOBOL, num_trials=5), + ] + ) + gs._nodes = [] + result = analysis.validate_applicable_state( + experiment=None, + generation_strategy=gs, + ) + self.assertIsNotNone(result) + self.assertIn("no nodes to visualize", result) def test_validate_applicable_state_valid(self) -> None: """Test that validation passes with a valid GenerationStrategy.""" diff --git a/ax/analysis/healthcheck/baseline_improvement.py b/ax/analysis/healthcheck/baseline_improvement.py index 0540afb0879..0fd20b71336 100644 --- a/ax/analysis/healthcheck/baseline_improvement.py +++ b/ax/analysis/healthcheck/baseline_improvement.py @@ -104,9 +104,8 @@ def validate_applicable_state( if experiment.optimization_config is None: return ( - "Experiment does not have an `OptimizationConfig`. " - "BaselineImprovementAnalysis requires defined objectives to compare " - "against for proper evaluation." + "The experiment must have an OptimizationConfig with defined " + "objectives to evaluate baseline improvement." ) try: diff --git a/ax/analysis/healthcheck/predictable_metrics.py b/ax/analysis/healthcheck/predictable_metrics.py index 9f2dd342b81..5b76524eec4 100644 --- a/ax/analysis/healthcheck/predictable_metrics.py +++ b/ax/analysis/healthcheck/predictable_metrics.py @@ -115,7 +115,10 @@ def validate_applicable_state( return experiment_validation if generation_strategy is None: - return "PredictableMetricsAnalysis requires a GenerationStrategy." + return ( + "A GenerationStrategy must be provided to evaluate metric " + "predictability." + ) # Resolve adapter from generation strategy if not provided resolved_adapter = adapter @@ -129,8 +132,9 @@ def validate_applicable_state( # RandomAdapter has no model to evaluate if isinstance(resolved_adapter, RandomAdapter): return ( - "PredictableMetricsAnalysis is not applicable when using a " - "RandomAdapter because there is no model to evaluate." + "This analysis is not applicable when using a random exploration " + "strategy (RandomAdapter) because there is no predictive model to " + "evaluate." ) return None diff --git a/ax/analysis/healthcheck/tests/test_predictable_metrics.py b/ax/analysis/healthcheck/tests/test_predictable_metrics.py index a538d0c52f2..a4e91c5c6b4 100644 --- a/ax/analysis/healthcheck/tests/test_predictable_metrics.py +++ b/ax/analysis/healthcheck/tests/test_predictable_metrics.py @@ -210,7 +210,7 @@ def test_random_adapter_not_applicable(self) -> None: self.assertIsNotNone(error) self.assertIn("RandomAdapter", error) - self.assertIn("no model to evaluate", error) + self.assertIn("no predictive model to evaluate", error) def test_validate_applicable_state_no_experiment(self) -> None: """Test that validation fails when experiment is None.""" diff --git a/ax/analysis/insights.py b/ax/analysis/insights.py index 08c6f634ee3..3c890d929e8 100644 --- a/ax/analysis/insights.py +++ b/ax/analysis/insights.py @@ -88,7 +88,10 @@ def validate_applicable_state( experiment = none_throws(experiment) if experiment.optimization_config is None: - return "Experiment must have an OptimizationConfig to compute insights." + return ( + "The experiment must have an OptimizationConfig (with defined " + "objectives) to compute insights." + ) @override def compute( @@ -191,14 +194,23 @@ def validate_applicable_state( experiment = none_throws(experiment) if experiment.optimization_config is None: - return "Experiment must have an OptimizationConfig." + return ( + "The experiment must have an OptimizationConfig (with defined " + "objectives and constraints)." + ) optimization_config = none_throws(experiment.optimization_config) if len(optimization_config.objective.metric_names) > 1: - return "Experiment may not have more than one Objective." + return ( + "This analysis only supports single-objective optimization. The " + "experiment has more than one objective metric." + ) if len(optimization_config.outcome_constraints) == 0: - return "Experiment must have at least one OutcomeConstraint." + return ( + "The experiment must have at least one OutcomeConstraint (a metric " + "constraint defined in the optimization config)." + ) @override def compute( diff --git a/ax/analysis/plotly/marginal_effects.py b/ax/analysis/plotly/marginal_effects.py index 24902e5fee7..1140b799708 100644 --- a/ax/analysis/plotly/marginal_effects.py +++ b/ax/analysis/plotly/marginal_effects.py @@ -96,8 +96,9 @@ def validate_applicable_state( ] if len(self.parameters) == 0: return ( - "MarginalEffectsPlot is only for `ChoiceParameter`s, " - "but no ChoiceParameters were found in the experiment." + "MarginalEffectsPlot is only applicable to experiments with " + "ChoiceParameters (discrete, enumerated parameters), but none " + "were found." ) else: for param_name in none_throws(self.parameters): diff --git a/ax/analysis/plotly/p_feasible.py b/ax/analysis/plotly/p_feasible.py index a20325f8f15..a968e85cec4 100644 --- a/ax/analysis/plotly/p_feasible.py +++ b/ax/analysis/plotly/p_feasible.py @@ -84,7 +84,10 @@ def validate_applicable_state( optimization_config = experiment.optimization_config if optimization_config is None: - return "Experiment must have an OptimizationConfig." + return ( + "The experiment must have an OptimizationConfig (with defined " + "objectives and constraints) to compute probability of feasibility." + ) if optimization_config.objective.is_multi_objective: return "Only single-objective optimization is supported." diff --git a/ax/analysis/plotly/tests/test_bandit_rollout.py b/ax/analysis/plotly/tests/test_bandit_rollout.py index 0b56f12de6e..444649f804a 100644 --- a/ax/analysis/plotly/tests/test_bandit_rollout.py +++ b/ax/analysis/plotly/tests/test_bandit_rollout.py @@ -39,7 +39,7 @@ def setUp(self) -> None: def test_validate_applicable_state(self) -> None: self.assertIn( - "Requires an Experiment", + "An Experiment must be provided", none_throws(BanditRollout().validate_applicable_state()), ) diff --git a/ax/analysis/plotly/tests/test_constraint_feasibility.py b/ax/analysis/plotly/tests/test_constraint_feasibility.py index f8dfe6086c6..cf0ae7c1358 100644 --- a/ax/analysis/plotly/tests/test_constraint_feasibility.py +++ b/ax/analysis/plotly/tests/test_constraint_feasibility.py @@ -24,7 +24,7 @@ class TestConstraintFeasibilityPlot(TestCase): def test_validate_applicable_state(self) -> None: self.assertIn( - "Requires an Experiment", + "An Experiment must be provided", none_throws(ConstraintFeasibilityPlot().validate_applicable_state()), ) diff --git a/ax/analysis/plotly/tests/test_marginal_effects.py b/ax/analysis/plotly/tests/test_marginal_effects.py index 57970e98f3d..39dc29a791d 100644 --- a/ax/analysis/plotly/tests/test_marginal_effects.py +++ b/ax/analysis/plotly/tests/test_marginal_effects.py @@ -75,7 +75,7 @@ def setUp(self) -> None: def test_validate_applicable_state(self) -> None: self.assertIn( - "Requires an Experiment", + "An Experiment must be provided", none_throws( MarginalEffectsPlot(metric_name="foo").validate_applicable_state() ), diff --git a/ax/analysis/plotly/tests/test_objective_p_feasible_frontier.py b/ax/analysis/plotly/tests/test_objective_p_feasible_frontier.py index 42e67c0e945..57777bc875c 100644 --- a/ax/analysis/plotly/tests/test_objective_p_feasible_frontier.py +++ b/ax/analysis/plotly/tests/test_objective_p_feasible_frontier.py @@ -155,7 +155,7 @@ def test_compute(self) -> None: def test_validate_applicable_state(self) -> None: self.assertIn( - "Requires an Experiment.", + "An Experiment must be provided", none_throws(ObjectivePFeasibleFrontierPlot().validate_applicable_state()), ) diff --git a/ax/analysis/plotly/tests/test_p_feasible.py b/ax/analysis/plotly/tests/test_p_feasible.py index b5c5e7e795b..e13fc63a7aa 100644 --- a/ax/analysis/plotly/tests/test_p_feasible.py +++ b/ax/analysis/plotly/tests/test_p_feasible.py @@ -24,10 +24,23 @@ class TestPFeasiblePlot(TestCase): def test_validate_applicable_state(self) -> None: self.assertIn( - "Requires an Experiment", + "An Experiment must be provided", none_throws(PFeasiblePlot().validate_applicable_state()), ) + self.assertIn( + "must have an OptimizationConfig", + none_throws( + PFeasiblePlot().validate_applicable_state( + experiment=get_branin_experiment( + with_trial=True, + with_completed_trial=True, + has_optimization_config=False, + ) + ) + ), + ) + self.assertIn( "must have at least one OutcomeConstraint", none_throws( diff --git a/ax/analysis/plotly/tests/test_parallel_coordinates.py b/ax/analysis/plotly/tests/test_parallel_coordinates.py index b0a8ebb371e..de2e921d654 100644 --- a/ax/analysis/plotly/tests/test_parallel_coordinates.py +++ b/ax/analysis/plotly/tests/test_parallel_coordinates.py @@ -26,7 +26,7 @@ class TestParallelCoordinatesPlot(TestCase): def test_validate_applicable_state(self) -> None: self.assertIn( - "Requires an Experiment", + "An Experiment must be provided", none_throws(ParallelCoordinatesPlot("branin").validate_applicable_state()), ) diff --git a/ax/analysis/plotly/tests/test_progression.py b/ax/analysis/plotly/tests/test_progression.py index 939358f5c38..fa0cab75cc7 100644 --- a/ax/analysis/plotly/tests/test_progression.py +++ b/ax/analysis/plotly/tests/test_progression.py @@ -23,7 +23,7 @@ class TestProgression(TestCase): def test_validate_applicable_state(self) -> None: self.assertIn( - "Requires an Experiment", + "An Experiment must be provided", none_throws( ProgressionPlot(metric_name="branin_map").validate_applicable_state() ), diff --git a/ax/analysis/plotly/tests/test_top_surfaces.py b/ax/analysis/plotly/tests/test_top_surfaces.py index dabae432915..97e7e8d9861 100644 --- a/ax/analysis/plotly/tests/test_top_surfaces.py +++ b/ax/analysis/plotly/tests/test_top_surfaces.py @@ -20,7 +20,7 @@ class TestTopSurfacesAnalysis(TestCase): def test_validate_applicable_state(self) -> None: self.assertIn( - "Requires an Experiment", + "An Experiment must be provided", none_throws(TopSurfacesAnalysis().validate_applicable_state()), ) diff --git a/ax/analysis/tests/test_metric_summary.py b/ax/analysis/tests/test_metric_summary.py index 0c16ccd1bd8..247ea276808 100644 --- a/ax/analysis/tests/test_metric_summary.py +++ b/ax/analysis/tests/test_metric_summary.py @@ -79,7 +79,7 @@ def test_compute(self) -> None: def test_validate_applicable_state(self) -> None: self.assertIn( - "Requires an Experiment", + "An Experiment must be provided", none_throws(MetricSummary().validate_applicable_state()), ) diff --git a/ax/analysis/tests/test_overview.py b/ax/analysis/tests/test_overview.py index 51837405151..dbd145f02c9 100644 --- a/ax/analysis/tests/test_overview.py +++ b/ax/analysis/tests/test_overview.py @@ -12,7 +12,11 @@ import pandas as pd from ax.adapter.base import Adapter from ax.adapter.registry import Generators -from ax.analysis.insights import _MAX_NUM_PARAMS_FOR_SECOND_ORDER, InsightsAnalysis +from ax.analysis.insights import ( + _MAX_NUM_PARAMS_FOR_SECOND_ORDER, + InsightsAnalysis, + OutcomeConstraintsAnalysis, +) from ax.analysis.overview import OverviewAnalysis from ax.analysis.plotly.arm_effects import ArmEffectsPlot from ax.analysis.plotly.scatter import ScatterPlot @@ -40,6 +44,8 @@ from ax.utils.common.constants import Keys from ax.utils.common.testutils import TestCase from ax.utils.testing.core_stubs import ( + get_branin_experiment, + get_branin_experiment_with_multi_objective, get_data, get_experiment_with_scalarized_objective_and_outcome_constraint, get_offline_experiments_subset, @@ -500,3 +506,50 @@ def patched_init(self_inner: TopSurfacesAnalysis, **kwargs: object) -> None: self.assertGreater(len(captured_orders), 0) for order in captured_orders: self.assertEqual(order, "total") + + def test_insights_validate_no_optimization_config(self) -> None: + """Test InsightsAnalysis validation when experiment has no + OptimizationConfig.""" + experiment = get_branin_experiment(has_optimization_config=False) + result = InsightsAnalysis().validate_applicable_state( + experiment=experiment, + ) + self.assertIsNotNone(result) + self.assertIn("must have an OptimizationConfig", result) + + def test_outcome_constraints_validate_applicable_state(self) -> None: + """Test OutcomeConstraintsAnalysis validation for various invalid + experiment configurations.""" + analysis = OutcomeConstraintsAnalysis() + + with self.subTest("no optimization config"): + result = analysis.validate_applicable_state( + experiment=get_branin_experiment( + with_trial=True, + with_completed_trial=True, + has_optimization_config=False, + ), + ) + self.assertIsNotNone(result) + self.assertIn("must have an OptimizationConfig", result) + + with self.subTest("multi-objective"): + result = analysis.validate_applicable_state( + experiment=get_branin_experiment_with_multi_objective( + with_trial=True, + with_completed_trial=True, + ), + ) + self.assertIsNotNone(result) + self.assertIn("only supports single-objective", result) + + with self.subTest("no outcome constraints"): + result = analysis.validate_applicable_state( + experiment=get_branin_experiment( + with_trial=True, + with_completed_trial=True, + has_optimization_config=True, + ), + ) + self.assertIsNotNone(result) + self.assertIn("must have at least one OutcomeConstraint", result) diff --git a/ax/analysis/tests/test_results.py b/ax/analysis/tests/test_results.py index 5282bc5736f..6f78821b02f 100644 --- a/ax/analysis/tests/test_results.py +++ b/ax/analysis/tests/test_results.py @@ -69,7 +69,7 @@ def test_validate_applicable_state(self) -> None: with self.subTest("requires_experiment"): error_message = analysis.validate_applicable_state() self.assertIsNotNone(error_message) - self.assertIn("Requires an Experiment", none_throws(error_message)) + self.assertIn("An Experiment must be provided", none_throws(error_message)) with self.subTest("requires_trials"): experiment = get_branin_experiment() diff --git a/ax/analysis/tests/test_utils.py b/ax/analysis/tests/test_utils.py index bde31c42f37..fbec00f981a 100644 --- a/ax/analysis/tests/test_utils.py +++ b/ax/analysis/tests/test_utils.py @@ -15,6 +15,7 @@ _prepare_p_feasible, _relativize_df_with_sq, prepare_arm_data, + validate_outcome_constraints, ) from ax.api.client import Client from ax.api.configs import RangeParameterConfig @@ -881,6 +882,17 @@ def test_offline(self) -> None: additional_arms=additional_arms, ) + def test_validate_outcome_constraints_no_optimization_config(self) -> None: + """Test validate_outcome_constraints returns error when experiment has + no OptimizationConfig.""" + experiment = Experiment( + name="no_opt_config", + search_space=self.client._experiment.search_space.clone(), + ) + result = validate_outcome_constraints(experiment=experiment) + self.assertIsNotNone(result) + self.assertIn("must have an OptimizationConfig", result) + def test_scalarized_constraints(self) -> None: df = pd.DataFrame( { diff --git a/ax/analysis/utils.py b/ax/analysis/utils.py index 40e7cf4bd21..526d65628db 100644 --- a/ax/analysis/utils.py +++ b/ax/analysis/utils.py @@ -1066,7 +1066,7 @@ def validate_experiment( """ if experiment is None: - return "Requires an Experiment." + return "An Experiment must be provided to compute this analysis." if require_trials: if len(experiment.trials) == 0: @@ -1172,7 +1172,10 @@ def validate_outcome_constraints( """ optimization_config = experiment.optimization_config if optimization_config is None: - return "Experiment must have an OptimizationConfig." + return ( + "The experiment must have an OptimizationConfig (with defined objectives " + "and constraints) to compute outcome constraints." + ) outcome_constraint_metrics = [ outcome_constraint.metric_names[0] @@ -1180,8 +1183,9 @@ def validate_outcome_constraints( ] if len(outcome_constraint_metrics) == 0: return ( - "Experiment must have at least one OutcomeConstraint to calculate " - "probability of feasibility." + "The experiment must have at least one OutcomeConstraint (a metric " + "constraint defined in the optimization config) to calculate probability " + "of feasibility." ) return None