Refactor Objective, OutcomeConstraint to be expression-based; remove Metrics from OptimizationConfig#5000
Open
mpolson64 wants to merge 1 commit intofacebook:mainfrom
Open
Refactor Objective, OutcomeConstraint to be expression-based; remove Metrics from OptimizationConfig#5000mpolson64 wants to merge 1 commit intofacebook:mainfrom
mpolson64 wants to merge 1 commit intofacebook:mainfrom
Conversation
…Metrics from OptimizationConfig Summary: This diff is a major refactor. In this diff we remove Metric instances from both Objective and OutcomeConstraint (and transitively, also off OptimizationConfig) and instead add them directly to the Experiment. In their place, we use the "expression" syntax and Sympy parsing from the Ax API directly on these classes, allowing users to specify `Objective(expression="ne1 + 2*ne2")` or `OutcomeConstraint(expression="qps >= 7000")` directly. This has a number of benefits: * Dependency spread is contained. OptimizationConfig is now able to be loaded without loading the any specific Metric implementation * Major separation of concerns: OptimizationConfig is now solely concerned with the goals of the optimization and not also implicitly concerned with fetching logic * Bringing Expressions directly onto Objective and OutcomeConstraint allows us to deprecated ScalarizedObjective, MultiObjective, ScalarizedOutcomeConstraint, and ObjectiveThreshold * Massive simplification for eventual upstreaming of MultiTypeExperiment functionality into base Experiment This is also a necessary step in our storage redesign in order to make metrics "live" only in one place. See this doc for details: https://docs.google.com/document/d/1I8FSQJ05_WHXFHBtNaL3dTAdAbvKfADwMRlIdq6BEV8/edit?tab=t.0#heading=h.s4r4rlrnva07 The refactor touches several core modules, please focus review on these: * ax/core/objective.py * ax/core/outcome_constraint.py * ax/core/optimization_config.py * ax/core/experiment.py These changes mandate many many updates to callsites which is ballooning the size of the diff. Changes are summarized here. # Core changes: * Objective, OutcomeConstraint classes now takes an expression string (e.g., "accuracy", "-loss", "2*acc + recall", "acc, -loss") * Old-style constructors still work, but they discard the Metric after initialization * Objective.metric property no longer exists. If a user wants to get the specific metric they retrieve its name via get_names and pass that string into Experiment.get_metric(name: str) -> Metric * isinstance checks for MultiObective, ScalarizedObjective, etc are replaced with is_multi_objective and is_scalarized_objective properties * Experiment.tracking_metrics is a property not an attr, now experiment simply has a collection of Metrics called "metrics" and tracking_metrics is a property which returns the metrics not on the optimization config * **Parsing is directly lifted and shifted from ax/api** * **Both JSON and SQL encoders/decoders carefully reconstruct the pre-refactor structure, allowing existing storage to be unaffected** # Other changes ## **ax/adapter/** - **adapter_utils.py** - Functions updated to accept experiment metrics for lookup. - All extraction functions use metric names. - **base.py, torch.py, transforms/** - Updated to thread experiment metrics through extraction functions. - Metric access patterns updated. ## **ax/analysis/** - Updated all metric access patterns to use metric names, not Metric objects. ## **ax/api/** - **client.py** - Simplified metric overwriting logic: metrics only live on Experiment. - **utils/instantiation/from_string.py** - Simplified parsing: constructs Objective directly from expression string. ## **ax/benchmark/** - Updated objective construction to use expression-based interface. ## **ax/core/** - **experiment.py** - Experiment now maintains a flat list of metrics (`_metrics: dict[str, Metric]`). - No distinction between tracking and optimization metrics. - Objective and constraints reference metrics by name, not object. - **objectives.py** - Unified Objective class: now takes an expression string, supporting single, scalarized, and multi-objective optimization. - Deprecated MultiObjective and ScalarizedObjective classes; their functionality is merged into Objective. - All metric access is via metric names/weights parsed from the expression, not direct Metric objects. - **optimization_config.py** - Unified OptimizationConfig class: handles both single and multi-objective cases. - Removed MultiObjectiveOptimizationConfig. - All config logic uses expression-based Objective and OutcomeConstraint. - **outcome_constraint.py** - Unified OutcomeConstraint class: now takes an expression string, supporting scalarized constraints. - Removed ObjectiveThreshold and ScalarizedOutcomeConstraint classes. - All validation and access is via metric names/weights parsed from the expression. - **trial.py** - Updated to reference metrics via `objective.metric_names[0]` instead of `objective.metric.name`. - **utils.py** - Updated metric access patterns to use metric names. ## **ax/early_stopping/** - Updated to reference metric signatures via experiment metrics, not via Objective. ## **ax/plot/** - Updated metric access patterns to use metric names. ## **ax/service/** - **ax_client.py** - Updated to reference metrics via `objective.metric_names[0]`. - Simplified metric overwriting logic. - **utils/best_point.py** - Updated metric access patterns to use metric names. ## **ax/storage/json_store/** - Encoder: serializes objectives as expression strings. - Decoder: converts legacy formats (with embedded Metric dicts) to expression-based objects. ## **ax/storage/sqa_store/** - Encoder: stores expression string in JSON column. - Decoder: reconstructs expression strings from MetricIntent-based SQA rows. ## **Tests** - All test constructors rewritten to use expression strings. - Assertions updated for new property names. - Added tests for expression parsing, multi-objective, scalarized, and minimize detection. Differential Revision: D93520819
|
@mpolson64 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D93520819. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
This diff is a major refactor.
In this diff we remove Metric instances from both Objective and OutcomeConstraint (and transitively, also off OptimizationConfig) and instead add them directly to the Experiment. In their place, we use the "expression" syntax and Sympy parsing from the Ax API directly on these classes, allowing users to specify
Objective(expression="ne1 + 2*ne2")orOutcomeConstraint(expression="qps >= 7000")directly. This has a number of benefits:This is also a necessary step in our storage redesign in order to make metrics "live" only in one place. See this doc for details: https://docs.google.com/document/d/1I8FSQJ05_WHXFHBtNaL3dTAdAbvKfADwMRlIdq6BEV8/edit?tab=t.0#heading=h.s4r4rlrnva07
The refactor touches several core modules, please focus review on these:
These changes mandate many many updates to callsites which is ballooning the size of the diff. Changes are summarized here.
Core changes:
Other changes
ax/adapter/
ax/analysis/
ax/api/
ax/benchmark/
ax/core/
_metrics: dict[str, Metric]).objective.metric_names[0]instead ofobjective.metric.name.ax/early_stopping/
ax/plot/
ax/service/
objective.metric_names[0].ax/storage/json_store/
ax/storage/sqa_store/
Tests
Differential Revision: D93520819