Skip to content

Conversation

esantorella
Copy link
Contributor

Summary:
Context:

TLDR: I am cleaning up and refactoring best-point functionality. n_points > 1 is not supported, so it doesn't make sense to refactor it.

  • Current state: Currently, the way we get a best point in benchmarking, is opaque because a recommendation is initially written to GeneratorRun, then parsed by BestPointMixin._get_best_trial, and then some more processing is done to ensure that the point is not None. This is OK for comparing different generation strategies, because it ensures that a best point will always be present. However, it not very helpful for comparing best-point selection itself. The right way to change a best-point recommendation is via GeneratorRun.best_arm_predictions, but all the post-processing will make it hard to see the effect of that. We initially wrote BenchmarkMethod.get_best_parameters with the expectation that users could subclass BenchmarkMethod and override get_best_parameters in order to control best-point selection more directly. But subclassing BenchmarkMethod is a bad idea that would cause serialization issues, and developing a best-point selector in that way would then require the work be redone to integrate it into Ax.
  • Intermediate goal state: Benchmarks simply use best-point predictions from GeneratorRun.best_arm_predictions. If this is None, the inference trace will be NaN at that point.
  • Ideal end state: Best-point selectors are eventually introduced as an Ax modeling abstraction that can be changed and run independently of candidate generation.

Differential Revision: D76159679

Summary:
*Context:*

TLDR: I am cleaning up and refactoring best-point functionality. `n_points > 1` is not supported, so it doesn't make sense to refactor it.

* Current state: Currently, the way we get a best point in benchmarking, is opaque because a recommendation is initially written to `GeneratorRun`, then parsed by `BestPointMixin._get_best_trial`, and then some more processing is done to ensure that the point is not None. This is OK for comparing different generation strategies, because it ensures that a best point will always be present. However, it not very helpful for comparing best-point selection itself. The right way to change a best-point recommendation is via `GeneratorRun.best_arm_predictions`, but all the post-processing will make it hard to see the effect of that. We initially wrote `BenchmarkMethod.get_best_parameters` with the expectation that users could subclass `BenchmarkMethod` and override `get_best_parameters` in order to control best-point selection more directly. But subclassing `BenchmarkMethod` is a bad idea that would cause serialization issues, and developing a best-point selector in that way would then require the work be redone to integrate it into Ax.
* Intermediate goal state: Benchmarks simply use best-point predictions from `GeneratorRun.best_arm_predictions`. If this is None, the inference trace will be NaN at that point.
* Ideal end state: Best-point selectors are eventually introduced as an Ax modeling abstraction that can be changed and run independently of candidate generation.

Differential Revision: D76159679
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jun 12, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D76159679

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants