Repository navigation
Ship 0.15.2 patches to main - #962
Open
AdrianSosic wants to merge 18 commits into
Open
AdrianSosic wants to merge 18 commits into
AdrianSosic wants to merge 18 commits into
Conversation
The fix restores the candidate identities but leaks the opaque `object()` carrier into `rec.index.name`, which becomes an `object` instance instead of `material_id`. The added assertions expose this leakage and currently fail.
The opaque `object()` index carrier leaked into `rec.index.name`. Use the `_df_index` carrier known from `filter_df` instead and restore the candidate index name in the discrete and hybrid path.
The discrete and hybrid variants shared their setup. Use a single parametrized test with a common candidate fixture and restrict the assertions to the candidate identities.
Does not yet exist on the patched version
### Summary Preserve named candidate identities in discrete and hybrid BoTorch recommendation result reconstruction. ### Details and comments When a search space is built from a dataframe with a named sample index (e.g. `material_id`), discrete recommendation crashes with `KeyError: 'index'` and hybrid discrete/continuous recommendation with `KeyError: "None of ['index'] are in the columns"`. Both consumers call `candidates_comp.reset_index()` then look up the literal column `index`, but pandas uses the original index name instead. The fix uses a local opaque column label for the index carrier in the existing reset-index/merge reconstruction, so it cannot collide with parameter or computational column labels, and reads that same carrier in both discrete and hybrid paths. Adds two regressions (discrete + hybrid) using real GP/BoTorch/qLogEI acquisition with named string indices, asserting configuration-to-sample identity mapping. Existing FAST_OPTIMIZATION_SETTINGS bound the work. Verified locally: `pytest -q tests/test_campaign.py` 68 passed including both new regressions; ruff/format/flake8/pydoclint pass on changed files. AI-assisted implementation; no human-review claim beyond this record.
`Campaign.parameters` also contains continuous parameters, so relying on the surrounding `DISCRETE` guard made the block fragile. The discrete accessor is also an existing property, making the comprehension redundant.
Both tests only differed in the search space. The unified case asserts all columns and the index name instead of a single column.
### Summary Preserve candidate identities when excluding pending experiments. ### Details and comments `Campaign.recommend` merged discrete candidate rows with pending experiments, producing a fresh `RangeIndex` and potentially multiple rows per candidate. That indexed merge result was OR-ed with metadata carrying the original candidate indices and applied as a positional filter, so supported `SearchSpace.from_dataframe` inputs with sample IDs could return a pending experiment again (even with `allow_recommending_pending_experiments=False`) or raise `NotEnoughPointsLeftError` although valid candidates remained. The fix replaces the merge-indicator Series with exact joint-row membership over consistently ordered discrete parameter columns, yielding one positional boolean per candidate regardless of sample index or duplicate pending rows. Existing measured/recommended/excluded metadata restrictions are combined unchanged; off-grid/tolerance behavior and rejection of empty pending dataframes are untouched. Adds regressions covering string/named/reordered numeric indices, duplicate pending rows, the allow-pending flag, nonmatching pending data and mixed categorical/numeric row matching. Verified locally: `pytest -q tests/test_campaign.py` 72 passed; ruff/format/flake8/pydoclint pass on changed files. AI-assisted implementation; no human-review claim beyond this record.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The hybrid index regression bypasses the required fast-test recommender configuration, causing unnecessary optimization work.
1 open issue
What changed in this PR
Brings the 0.15.2 recommendation patches from #958 and #959 to main, addressing candidate-index handling in BayBE’s recommendation pipeline.
Changes:
- Preserve named candidate indices during discrete and hybrid BoTorch result reconstruction.
- Exclude pending discrete configurations without misaligning custom indices or duplicating candidates.
- Add regression coverage, document the release, and enable CI for patch branches.
| File | Description |
|---|---|
tests/test_campaign.py |
Adds pending-experiment filtering and named-index regressions. |
CHANGELOG.md |
Documents the two 0.15.2 fixes. |
baybe/recommenders/pure/bayesian/botorch/hybrid.py |
Preserves named indices during hybrid result reconstruction. |
baybe/recommenders/pure/bayesian/botorch/discrete.py |
Preserves named indices during discrete result reconstruction. |
baybe/campaign.py |
Filters pending experiments using exact joint-row membership. |
.github/workflows/ci.yml |
Enables CI for patch/** branches. |
Review effort: Balanced
This branch has not been deployed
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.
Uh oh!
There was an error while loading. Please reload this page.