Skip to content

Commit

Permalink
Get rid of SurrogateRunner and SurrogateBenchmarkProblem (facebook#2954)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#2954

**Context**:  This diff cuts over over uses of `SurrogateRunner` to use `ParamBasedTestProblemRunner` with a `test_problem` that is the newly introduced `SurrogateTestFunction`, and the following diff after that will bring us down to only one runner class for benchmarking by merging `ParamBasedTestProblemRunner` into `BenchmarkRunner`. Having only one runner will make it easier to enable asynchronous benchmarks. Currently, SurrogateRunner had its own logic for tracking when trials are completed, which would make it difficult to work in with asynchronicity.

**Note on naming**: Some names have become non-intuitive in the process of benchmarking. To accord with some future changes I hope to make, I called a new class SurrogateTestFunction, whereas SurrogateParamBasedTestProblem would be more in line with the old naming.

The name changes I hope to make:
* ParamBasedTestProblemRunner -> nothing, absorbed into BenchmarkRunner
* ParamBasedTestProblem -> TestFunction, to emphasize that all it does is generate data (rather than more generally specify the problem we are solving) and that it is deterministic, and to differentiate it from BenchmarkProblem. BenchmarkTestFunction would also be a candidate.
* BoTorchTestProblem -> BoTorchTestFunction

**Changes in this diff**:
* Introduces SurrogateTestFunction, a ParamBasedTestProblem for * Removes SurrogateRunner. There is some loss of (unused) functionality here; dict-valued 'noise_std' is no longer supported. I expect we will eventually want that, but it doesn't need to be done now.
* Changed `predict_for_tensor` utility function to use a `SurrogateTestFunction` rather than a `SurrogateRunner`
* Removed `SurrogateRunner` from the json_store registry. There is no point in keeping it around for backward compatibility since it was just deserializing to a dummy `SyntheticRunner` anyway.

Differential Revision: D64855010
  • Loading branch information
esantorella authored and facebook-github-bot committed Oct 24, 2024
1 parent a35c3e0 commit f8bb4ae
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 75 deletions.
33 changes: 0 additions & 33 deletions ax/benchmark/problems/surrogate.py

This file was deleted.

2 changes: 0 additions & 2 deletions ax/storage/json_store/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,6 @@
"SumConstraint": SumConstraint,
"Surrogate": Surrogate,
"SurrogateMetric": BenchmarkMetric, # backward-compatiblity
# NOTE: SurrogateRunners -> SyntheticRunner on load due to complications
"SurrogateRunner": SyntheticRunner,
"SobolQMCNormalSampler": SobolQMCNormalSampler,
"SyntheticRunner": SyntheticRunner,
"SurrogateSpec": SurrogateSpec,
Expand Down
49 changes: 9 additions & 40 deletions ax/utils/testing/benchmark_stubs.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@
from ax.benchmark.benchmark_metric import BenchmarkMetric
from ax.benchmark.benchmark_problem import BenchmarkProblem, create_problem_from_botorch
from ax.benchmark.benchmark_result import AggregatedBenchmarkResult, BenchmarkResult
from ax.benchmark.problems.surrogate import SurrogateBenchmarkProblem
from ax.benchmark.runners.botorch_test import (
ParamBasedTestProblem,
ParamBasedTestProblemRunner,
)
from ax.benchmark.runners.surrogate import SurrogateRunner, SurrogateTestFunction
from ax.benchmark.runners.surrogate import SurrogateTestFunction
from ax.core.experiment import Experiment
from ax.core.objective import MultiObjective, Objective
from ax.core.optimization_config import (
Expand Down Expand Up @@ -129,41 +128,7 @@ def get_soo_surrogate() -> BenchmarkProblem:
)


def get_soo_surrogate_legacy() -> SurrogateBenchmarkProblem:
experiment = get_branin_experiment(with_completed_trial=True)
surrogate = TorchModelBridge(
experiment=experiment,
search_space=experiment.search_space,
model=BoTorchModel(surrogate=Surrogate(botorch_model_class=SingleTaskGP)),
data=experiment.lookup_data(),
transforms=[],
)
runner = SurrogateRunner(
name="test",
outcome_names=["branin"],
get_surrogate_and_datasets=lambda: (surrogate, []),
)

observe_noise_sd = True
objective = Objective(
metric=BenchmarkMetric(
name="branin", lower_is_better=True, observe_noise_sd=observe_noise_sd
),
)
optimization_config = OptimizationConfig(objective=objective)

return SurrogateBenchmarkProblem(
name="test",
search_space=experiment.search_space,
optimization_config=optimization_config,
num_trials=6,
observe_noise_stds=observe_noise_sd,
optimal_value=0.0,
runner=runner,
)


def get_moo_surrogate() -> SurrogateBenchmarkProblem:
def get_moo_surrogate() -> BenchmarkProblem:
experiment = get_branin_experiment_with_multi_objective(with_completed_trial=True)
surrogate = TorchModelBridge(
experiment=experiment,
Expand All @@ -173,11 +138,15 @@ def get_moo_surrogate() -> SurrogateBenchmarkProblem:
transforms=[],
)

runner = SurrogateRunner(
test_function = SurrogateTestFunction(
num_objectives=2,
name="test",
outcome_names=["branin_a", "branin_b"],
get_surrogate_and_datasets=lambda: (surrogate, []),
)
runner = ParamBasedTestProblemRunner(
test_problem=test_function,
outcome_names=["branin_a", "branin_b"],
)
observe_noise_sd = True
optimization_config = MultiObjectiveOptimizationConfig(
objective=MultiObjective(
Expand All @@ -199,7 +168,7 @@ def get_moo_surrogate() -> SurrogateBenchmarkProblem:
],
)
)
return SurrogateBenchmarkProblem(
return BenchmarkProblem(
name="test",
search_space=experiment.search_space,
optimization_config=optimization_config,
Expand Down

0 comments on commit f8bb4ae

Please sign in to comment.