Fix bounds indexing and candidate names for contextual BO with fixed features#426
Merged
roussel-ryan merged 3 commits intoJun 3, 2026
Conversation
- _candidate_names: copy variable list to avoid mutation, exclude contextual variables - _get_optimization_bounds: compute keep-indices once from original list, slice bounds once - vocs.py: update error message to mention non-contextual variables
Copilot
AI
changed the title
[WIP] Fix code based on review comments
Fix bounds indexing and candidate names for contextual BO with fixed features
Jun 3, 2026
Collaborator
|
@copilot add tests to validate the new behavior |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- test_candidate_names_excludes_contextual_and_fixed: verifies _candidate_names excludes both contextual variables and fixed features, and does not mutate vocs - test_optimization_bounds_excludes_contextual_and_fixed: verifies _get_optimization_bounds correctly handles combined fixed_features + contextual_variables in a single pass - test_convert_dataframe_to_inputs_error_message_contextual: verifies the updated error message mentions 'non-contextual' variables
Contributor
Author
Added three targeted tests in
|
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.
When both
fixed_featuresandcontextual_variablesare active, the generator double-sliced the bounds tensor using stale indices from the original variable list, causing out-of-range indexing and wrong column drops. Similarly,_candidate_namesmutatedvocs.variable_namesin place and omitted contextual variables, causing column mismatches in_process_candidates.Changes
_get_optimization_bounds: Replaced two sequential index+slice passes with a single exclusion set (fixed_features ∪ contextual_variables) computed against the original variable list, slicingboundsonce._candidate_names: Copy the variable list before filtering (avoids mutating VOCS state), and exclude contextual variables alongside fixed features so the returned names always match the optimized tensor dimensions.vocs.pyerror message: Clarified the validation error to say "non-contextual vocs variables" — the old message said "vocs variables", which was misleading since contextual variables are intentionally excluded from the expected column set.xopt/tests/generators/bayesian/test_contextual_bo.pyto validate the new behaviour:test_candidate_names_excludes_contextual_and_fixed— verifies exclusion of both contextual and fixed-feature variables and no mutation ofvocs.variable_names.test_optimization_bounds_excludes_contextual_and_fixed— verifies correct bounds shape and values when bothfixed_featuresand contextual variables are present.test_convert_dataframe_to_inputs_error_message_contextual— verifies the updated error message references "non-contextual" variables.