discrete variables#421
Conversation
add mggpo errors add tests initial commit
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@copilot add tests to this PR that ensures coverage of changes |
Agent-Logs-Url: https://github.com/xopt-org/Xopt/sessions/fb505abd-9eb7-46d5-803a-716cc9dd4a86 Co-authored-by: roussel-ryan <24279776+roussel-ryan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/xopt-org/Xopt/sessions/fb505abd-9eb7-46d5-803a-716cc9dd4a86 Co-authored-by: roussel-ryan <24279776+roussel-ryan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/xopt-org/Xopt/sessions/fb505abd-9eb7-46d5-803a-716cc9dd4a86 Co-authored-by: roussel-ryan <24279776+roussel-ryan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/xopt-org/Xopt/sessions/fb505abd-9eb7-46d5-803a-716cc9dd4a86 Co-authored-by: roussel-ryan <24279776+roussel-ryan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/xopt-org/Xopt/sessions/fb505abd-9eb7-46d5-803a-716cc9dd4a86 Co-authored-by: roussel-ryan <24279776+roussel-ryan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/xopt-org/Xopt/sessions/fb505abd-9eb7-46d5-803a-716cc9dd4a86 Co-authored-by: roussel-ryan <24279776+roussel-ryan@users.noreply.github.com>
Added targeted coverage tests for the discrete-variable changes in Bayesian generator/model-constructor and numerical optimizer paths, including truncation/warning branches and discrete validation/error cases. Addressed in commit 8ccd393. |
|
@copilot create an ipynb example demonstrating bayesian optimization in a mixed continuous-discrete space |
Agent-Logs-Url: https://github.com/xopt-org/Xopt/sessions/030adb28-b5e0-4dbf-9057-ed2c8db9ee89 Co-authored-by: roussel-ryan <24279776+roussel-ryan@users.noreply.github.com>
Added the notebook example at |
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end support for discrete (and mixed continuous/discrete) variables across Xopt’s Bayesian optimization workflow, including candidate generation, optimizer routing, validation, visualization, and documentation/examples.
Changes:
- Extend bounds/sampling/validation utilities to correctly handle discrete variables (
xopt/vocs.py) and enforce generator capability checks (supports_discrete_variables). - Add mixed/discrete optimization support to
LBFGSOptimizervia BoTorch’soptimize_acqf_mixed/optimize_acqf_discrete, and integrate discrete-aware candidate handling inBayesianGenerator. - Expand tests and docs (including a new mixed discrete tutorial notebook) to cover discrete workflows, plotting, and unsupported-generator errors.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| xopt/vocs.py | Adds discrete-aware variable bounds, random/grid sampling, and input validation. |
| xopt/tests/test_vocs.py | Adds unit tests for discrete sampling, grid enumeration, and validation membership. |
| xopt/numerical_optimizer.py | Adds mixed/discrete optimization routes and new configuration limits/logging. |
| xopt/tests/test_numerical_optimizer.py | Tests optimizer routing, truncation behavior, and argument handling for mixed/discrete paths. |
| xopt/generator.py | Introduces supports_discrete_variables capability flag + validation hook. |
| xopt/tests/test_generator.py | Adds tests verifying discrete-support validation behavior for generators. |
| xopt/generators/random.py | Declares RandomGenerator supports discrete variables. |
| xopt/generators/bayesian/bayesian_generator.py | Implements discrete optimization kwargs, candidate snapping, and discrete validation; blocks unsupported features. |
| xopt/tests/generators/bayesian/test_bayesian_generator.py | Adds tests for discrete kwargs generation, snapping/validation, and grid-optimizer error paths. |
| xopt/generators/bayesian/base_model.py | Ensures model constructors derive bounds correctly for discrete domains. |
| xopt/generators/bayesian/visualize.py | Adds discrete-aware meshes/axes ticks and 2D plotting adjustments for mixed spaces. |
| xopt/tests/generators/bayesian/test_visualize.py | Adds tests for mixed discrete visualization mesh generation and tick labeling. |
| xopt/generators/bayesian/mggpo.py | Explicitly declares MGGPO does not support discrete variables. |
| xopt/tests/generators/bayesian/test_mggpo.py | Adds test ensuring MGGPO rejects discrete VOCS. |
| xopt/generators/bayesian/time_dependent.py | Aligns capability flags and preserves inherited VOCS validation behavior. |
| xopt/generators/bayesian/bax_generator.py | Preserves inherited Bayesian VOCS validation via super(). |
| xopt/generators/init.py | Excludes supports_discrete_variables from generator defaults export. |
| xopt/tests/generators/bayesian/test_discrete_integration.py | New integration tests exercising discrete candidate flow across multiple generators. |
| environment.yml | Bumps BoTorch dependency minimum to enable new optimization routines. |
| mkdocs.yml | Adds nav links for mixed discrete optimization example(s). |
| docs/examples/single_objective_bayes_opt/mixed_discrete.ipynb | New tutorial notebook demonstrating mixed continuous/discrete BO usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| discrete_indices = sorted(discrete_values) | ||
| value_lists = [discrete_values[idx] for idx in discrete_indices] | ||
| combinations = list(product(*value_lists)) | ||
|
|
||
| if isinstance(self.numerical_optimizer, LBFGSOptimizer): | ||
| max_configs = self.numerical_optimizer.mixed_max_discrete_configurations | ||
| if len(combinations) > max_configs: | ||
| logger.warning( | ||
| "truncating discrete configuration count from %d to %d", | ||
| len(combinations), | ||
| max_configs, | ||
| ) | ||
| combinations = combinations[:max_configs] | ||
|
|
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Implemented the feedback by making discrete configuration generation bounded/lazy in _get_discrete_optimization_kwargs so we no longer materialize the full cartesian product before truncation. Also added a regression test to verify truncation is applied lazily. Addressed in commits 0f356db and 944f7b0.
| n : Union[int, Dict[str, int]] | ||
| Number of points to generate along each axis. If an integer is provided, the same number of points | ||
| is used for all variables. If a dictionary is provided, it should have variable names as keys and | ||
| the number of points as values. | ||
| Number of points to generate along each axis for continuous variables. If an integer is provided, | ||
| the same number of points is used for all continuous variables. If a dictionary is provided, it | ||
| should have continuous variable names as keys and the number of points as values. Discrete | ||
| variables always use their configured values (optionally filtered by custom bounds). | ||
| custom_bounds : dict, optional | ||
| Custom bounds for the variables. If None, the default bounds from `vocs.variables` are used. | ||
| The dictionary should have variable names as keys and a list of two values [min, max] as values. | ||
| For discrete variables, bounds are used to filter allowed values. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ialization Agent-Logs-Url: https://github.com/xopt-org/Xopt/sessions/475590fc-706f-42f5-bfcf-78c42bccb8d6 Co-authored-by: roussel-ryan <24279776+roussel-ryan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/xopt-org/Xopt/sessions/475590fc-706f-42f5-bfcf-78c42bccb8d6 Co-authored-by: roussel-ryan <24279776+roussel-ryan@users.noreply.github.com>
nikitakuklev
left a comment
There was a problem hiding this comment.
A few small things. We can rework numopt later to maybe subclass.
| return [values[0], values[-1]] | ||
| return variable.domain | ||
|
|
||
| def _get_active_discrete_variable_values(self) -> dict[int, list[float]]: |
There was a problem hiding this comment.
This scans only _candidate_names, not fixed features.
There was a problem hiding this comment.
candidate names should include fixed features
There was a problem hiding this comment.
this is because the generator also returns fixed feature values to Xopt
There was a problem hiding this comment.
Sorry, poor initial wording.
Your link has:
variable_names = self.vocs.variable_names
if self.fixed_features is not None:
for name in self.fixed_features:
if name in variable_names:
variable_names.remove(name)
return variable_names
Then idx, name in enumerate(self._candidate_names): does not contain the discrete variable - max_travel_distances/turbo_controller proceed into _get_max_travel_distances_region which raises AttributeError: 'DiscreteVariable' object has no attribute 'domain'.
Example:
vocs = VOCS(variables={"x1": [0.0, 1.0], "x2": {0.0, 5.0, 10.0}}, # x2 discrete
objectives={"y1": "MINIMIZE"})
gen = ExpectedImprovementGenerator(vocs=vocs, fixed_features={"x2": 5.0}) # pin x2
gen.max_travel_distances = [0.1, 0.1]
gen.add_data(data)
gen._get_optimization_bounds() # AttributeError: 'DiscreteVariable' has no attribute 'domain'
There was a problem hiding this comment.
ah I see, ok I will address
There was a problem hiding this comment.
BAX just snaps discrete values at the end - should it be marked as supported?
There was a problem hiding this comment.
I think we can keep supports_discrete_variables to False for now until @dylanmkennedy can investigate further since there may be some issue with GridScanAlgorithm or other algorithm types. This is certainly a topic of interest
There was a problem hiding this comment.
yes, I meant set explicitly false (it inherits right now)
|
Thanks for the feedback @nikitakuklev |
| ) | ||
| else: | ||
| assert len(result) == 1 | ||
| result = interpolate_points( |
There was a problem hiding this comment.
this will emit off-grid point now, we should disallow when discrete variables are present
| ) | ||
|
|
||
|
|
||
| def _get_variable_bounds(vocs: VOCS) -> dict[str, tuple[float, float]]: |
There was a problem hiding this comment.
single discrete value will crash Normalize with a nan
This pull request adds robust support for discrete variables in the Bayesian optimization workflow. It introduces logic to correctly handle discrete variables throughout candidate generation, optimization, and validation, and ensures that optimizers and generators provide appropriate errors or warnings when discrete variables are not supported. Additionally, it updates dependencies and expands test coverage for these new behaviors.
Support for discrete variables in Bayesian optimization:
Added logic in
bayesian_generator.pyto handle discrete variables, including:max_travel_distancesandturbo_controllerwith discrete variables ([xopt/generators/bayesian/bayesian_generator.pyR1018-R1027])Updated model construction to properly handle discrete variable bounds [1]], [2]])
Enhancements to numerical optimizers:
LBFGSOptimizerto support mixed and discrete optimization via BoTorch'soptimize_acqf_mixedandoptimize_acqf_discrete, including configuration limits and warnings [1]], [2]])Validation and error handling improvements:
MGGPOGeneratorto explicitly disallow discrete variables, raising a clear error if present [1]], [2]])Dependency and test updates:
botorchdependency to>=0.13.0for compatibility with new optimization routines ([environment.ymlL11-R11])References:
Support for discrete variables in Bayesian optimization: [1]], [2]], [3]], [4]], [5]], [6]], [7]], [8]])
Enhancements to numerical optimizers: [1]], [2]])
Validation and error handling improvements: [1]], [2]])
Dependency and test updates: [1]], [2]])