Refactor recommendation mechanics#838
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors internal recommendation plumbing to remove redundant discrete-candidate threading now that SubspaceDiscrete can be safely copied in a filtered form, simplifying the recommender call stack.
Changes:
- Removes
FilteredSubspaceDiscreteand switchesCampaignto create filtered subspace copies directly. - Drops dead
pending_experimentsplumbing into_recommend_with_discrete_partsand removes redundantcandidates_expparameters across multiple recommenders/helpers. - Updates BoTorch/non-predictive recommenders to consume candidates via
subspace_discrete.exp_rep/.comp_repdirectly.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| CHANGELOG.md | Notes the internal simplification/refactor. |
| baybe/searchspace/_filtered.py | Deletes FilteredSubspaceDiscrete (no longer needed). |
| baybe/campaign.py | Builds filtered discrete subspaces via attrs.evolve(..., exp_rep=...) instead of a special subclass. |
| baybe/recommenders/pure/base.py | Removes unused pending_experiments arg and stops threading candidates_exp. |
| baybe/recommenders/pure/bayesian/botorch/core.py | Updates dispatch to new helper signatures without candidates_exp. |
| baybe/recommenders/pure/bayesian/botorch/discrete.py | Switches subset handling to evolve-created filtered subspaces. |
| baybe/recommenders/pure/bayesian/botorch/hybrid.py | Removes candidates_exp plumbing and relies on the (possibly filtered) discrete subspace. |
| baybe/recommenders/pure/nonpredictive/sampling.py | Uses searchspace.discrete.exp_rep / subspace_discrete.exp_rep directly. |
| baybe/recommenders/pure/nonpredictive/clustering.py | Uses subspace_discrete.exp_rep / .comp_rep directly. |
| baybe/recommenders/naive.py | Stops building/passing separate discrete candidate frames into _recommend_discrete. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
myrazma
reviewed
Jun 24, 2026
fabianliebig
approved these changes
Jun 25, 2026
AVHopp
approved these changes
Jun 25, 2026
AVHopp
left a comment
Collaborator
There was a problem hiding this comment.
Two minor comments, but LGTM
myrazma
approved these changes
Jun 25, 2026
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.
This is a pure internal refactor — no behavior change.
Background
The consistency of the attribute structure of
SubspaceDiscretewas historically enforced only by convention, not by the type system, meaning that mutating a subspace instance could produce an inconsistent state. Creating filtered copies of a subspace was therefore not safe. On top of that, doing so could have been potentially costly, since it might have involved materializing (parts of) the Cartesian product — another reason to avoid it.As a workaround, the codebase avoided modifying subspace objects for filtered candidate sets altogether and instead computed candidates externally, threading them as a separate
candidates_expdataframe argument down through multiple layers of the recommender call stack. TheFilteredSubspaceDiscretesubclass existed for the same reason: to apply a Boolean mask at call time without touching the subspace itself.Previous preparatory PRs resolved issue #794 by making the consistency of the attribute structure of
SubspaceDiscretean enforced invariant, which now enables safe creation of filtered subspace copies. The cost problem — to be avoided by lazy materialization of the Cartesian product (see #796) — has not yet been addressed but will be resolved in a subsequent PR on the same development branch. This PR capitalizes on the invariant enforcement to simplify the recommender internals.Changes
Drop
FilteredSubspaceDiscrete(baybe/searchspace/_filtered.pydeleted)The subclass existed solely to override
get_candidates()with a Boolean mask applied at call time. Now that creating a filtered copy of a subspace is safe, the class is unnecessary.Campaignis updated to create filtered subspace copies directly.Drop dead
pending_experimentsargumentThe
pending_experimentsparameter had been used inPureRecommender._recommend_with_discrete_partsto filter pending experiments from the candidate set, but became dead code when that filtering responsibility was moved toCampaignin an earlier refactor. It is now removed.Drop redundant
candidates_expargumentThe filtered set of discrete candidates was fetched once and then passed as a parallel
candidates_expdataframe through several layers of the recommender stack (_recommend→_recommend_discrete/hybrid→ BoTorch-specific helpers). With safe subspace mutation now available, subset-constrained paths are updated to create filtered copies of the subspace instead of propagating a separate dataframe. The argument is removed from all affected signatures across:PureRecommender._recommend_discrete/_recommend_hybridBotorchRecommender._recommend_discrete/_recommend_hybridrecommend_discrete_with_subsets/recommend_discrete_without_subsetsrecommend_hybrid_with_subsets/recommend_hybrid_without_subsetsSKLearnClusteringRecommender._recommend_discreteFPSRecommender._recommend_discreteRandomRecommender._recommend_hybridNaiveHybridSpaceRecommender