GPSampler as Default Optuna Sampler - #47
Conversation
There was a problem hiding this comment.
Pull request overview
Updates MotherML’s Optuna tuning behavior by switching the default sampler selection logic and making Optuna early-termination safer for hold-out validation setups.
Changes:
-
Make
MotherTunerdefault tooptuna.samplers.GPSamplerwhen torch is available, otherwise fall back toTPESampler. -
Bump
mother-mlversion inuv.lockto1.0.1.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| uv.lock | Updates locked package version for mother-ml to 1.0.1. |
| src/mother/optimization/core.py | Changes default Optuna sampler selection and adds hold-out detection in early-stopping callback setup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…er-when-torch-is-installed
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/mother/optimization/core.py:152
- Default sampler selection switches to GPSampler whenever
torch_availableis true, buttorch_availableonly checks fortorchand does not handle missing GP dependencies (e.g.scipy) or other instantiation-time import errors. In environments where torch is installed but scipy is not,optuna.samplers.GPSampler(...)will raise at runtime instead of falling back to TPE. This also regresses the prior ability to override TPESampler'smultivariateviakwargs.
Consider wrapping GPSampler construction in a try/except and falling back to TPESampler, while preserving multivariate=kwargs.get("multivariate", True).
if torch_available:
module_logger.debug("torch available — using GPSampler as default")
self.sampler = optuna.samplers.GPSampler(
seed=seed,
n_startup_trials=n_startup_trials,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/mother/optimization/core.py:152
- Default sampler selection only checks
torch_available, butGPSamplerrequires additional optional deps (e.g. SciPy) and can raiseImportErrorat instantiation time even when torch is installed. This also contradicts the PR description which says it should fall back toTPESamplerwhen GP dependencies are unavailable. Consider wrappingGPSamplerinitialization intry/except ImportErrorand falling back, and restore the priormultivariateoverride viakwargs.get("multivariate", True)for backward compatibility.
if torch_available:
module_logger.debug("torch available — using GPSampler as default")
self.sampler = optuna.samplers.GPSampler(
seed=seed,
n_startup_trials=n_startup_trials,
src/mother/optimization/core.py:179
get_callbacksnow has across_validationparameter and can skip early stopping for hold-out (n_splits < 2), but the docstring still only describes the torch availability behavior. Updating the docstring will help callers understand whyNonemay be returned even when early stopping is enabled.
"""
Prepares and returns a list of callbacks for early stopping in Optuna optimization.
If early stopping with Optuna is enabled and PyTorch is available, this method
will return a list containing a TerminatorCallback instance. If PyTorch is not
Use a safe fallback to TPESampler when GPSampler initialization fails, even if torch is available. Preserve the multivariate override in the TPESampler path. Refactor sampler construction into dedicated builder methods for cleaner maintainability. Add tests covering GPSampler failure fallback and multivariate override behavior.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
test/unit/test_model_tuner.py:252
- This test forces torch_available=True but does not ensure torch/scipy are actually installed. Since torch is an optional extra (pyproject.toml [project.optional-dependencies].torch), GPSampler initialization can fail in CI and the tuner will correctly fall back to TPESampler, making this assertion flaky. Consider stubbing GPSampler to a lightweight fake so the test only validates branch selection logic.
monkeypatch.setattr("mother.optimization.core.torch_available", True)
tuner = MotherTuner(
scorer=make_scorer(mean_squared_error, greater_is_better=False),
)
assert isinstance(tuner.sampler, optuna.samplers.GPSampler)
Use GPSampler as the default sampler when torch is available, with safe fallback to TPESampler if GP initialization fails. Configure TPESampler as the independent sampler for GPSampler so conditional parameters fall back to history-informed sampling instead of pure random sampling. Fix sampler assignment in MotherTuner.init and skip TerminatorCallback for hold-out validation where early stopping is invalid. Add test coverage for default sampler selection, GP fallback behavior, and hold-out callback handling. No breaking changes.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
test/unit/test_model_tuner.py:248
- This test forces
torch_available=Truebut does not ensure the GP stack is actually installed. In environments where the optionaltorch/scipydependencies are not installed (they are not in the default dev dependency group),MotherTunerwill fall back toTPESamplerand this assertion will fail. Skip this test whentorch/scipyare unavailable (similar to the later GP test).
def test_gp_sampler_when_torch_available(self, monkeypatch):
"""GPSampler should be selected when torch is available."""
monkeypatch.setattr("mother.optimization.core.torch_available", True)
src/mother/optimization/core.py:138
_build_tpe_samplerhard-codesmultivariate=True/group=True/constant_liar=True. If callers previously relied on tuning these (e.g., disabling multivariate TPE for certain search spaces), there’s no longer a supported way to do so whensampler=None. Consider either (a) exposing these as explicitMotherTunerparameters, or (b) documenting that the defaults are now fixed andsampler=must be provided for customization.
multivariate=True,
group=True,
constant_liar=True,
seed=seed,
n_startup_trials=n_startup_trials,
use GPSampler as default sampler when torch is available configure TPESampler as independent_sampler for GPSampler keep robust fallback to TPESampler when GPSampler init fails or torch is unavailable remove hold-out-specific callback behavior from this branch update tests for default sampler selection and callback behavior
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/mother/optimization/core.py:151
_build_gp_samplerconfiguresindependent_sampleras a plainTPESampler(seed=..., n_startup_trials=...), which drops the stronger default TPE configuration used elsewhere (multivariate=True,group=True,constant_liar=True). That means parameters sampled outside the GP relative search space (and startup trials) may behave differently/worse than the intended TPE fallback.
return optuna.samplers.GPSampler(
seed=seed,
independent_sampler=optuna.samplers.TPESampler(
seed=seed,
n_startup_trials=n_startup_trials,
),
test/unit/test_model_tuner.py:254
- This test forces
torch_available=Truewithout ensuring the optional GP stack is actually installed. If torch (and any GPSampler runtime deps) are not present in the test environment,MotherTunerwill correctly fall back toTPESamplerand this assertion will fail. Skip when torch isn't installed (and consider also skipping when GPSampler deps are missing).
def test_gp_sampler_when_torch_available(self, monkeypatch):
"""GPSampler should be selected when torch is available."""
monkeypatch.setattr("mother.optimization.core.torch_available", True)
tuner = MotherTuner(
…nal deps
gate test_gp_sampler_when_torch_available with pytest.importorskip("torch")
gate test_gp_sampler_when_torch_available with pytest.importorskip("scipy")
align optional-dependency handling with existing test patterns
avoid false failures when optional extras are not installed
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
src/mother/optimization/core.py:181
MotherTuner.__init__still accepts**kwargs, but the default TPESampler configuration no longer honors the previously-supportedmultivariateoverride (it is now alwaysTrue). This is a silent behavioral/API change for callers that passmultivariate=expecting it to affect the default sampler.
if sampler is None:
if torch_available:
try:
module_logger.debug("torch available — using GPSampler as default")
self.sampler = self._build_gp_sampler(
src/mother/optimization/core.py:185
- The PR description says hold-out (single-fold) validation disables
TerminatorCallback, butoptimize()still unconditionally passescallbacks=self.get_callbacks()wheneverearly_stopping_optunais enabled andtorch_availableis true. With a 1-split CV object, this can still attach the terminator and potentially crash/behave incorrectly when only one CV score is reported.
if torch_available:
try:
module_logger.debug("torch available — using GPSampler as default")
self.sampler = self._build_gp_sampler(
seed=seed,
n_startup_trials=n_startup_trials,
)
except Exception as gpsampler_error:
test/unit/test_model_tuner.py:216
- This test forces
torch_available=Truevia monkeypatch but does not skip when PyTorch is actually not installed. In environments without torch, constructingTerminatorCallback()may fail (or the import path may differ), making the test suite brittle.
def test_early_stopping_enabled_returns_callback_when_torch_available(self, monkeypatch):
"""get_callbacks should return terminator callback when enabled and torch is available."""
monkeypatch.setattr("mother.optimization.core.torch_available", True)
tuner = MotherTuner(
Branch:
46-make-gp-sampler-the-standard-optuna-sampler-when-torch-is-installedOverview
This PR changes MotherML's default Optuna sampler selection when users do not pass a custom sampler.
GPSamplerTPESamplerGPSamplerinitialization fails for any reason: fall back toTPESamplerThe goal is to get stronger optimization in low-budget, expensive-trial settings, while keeping behavior robust across environments.
Sampler decision logic (plain-English)
When
sampler=None, MotherML now does the following:GPSampler.TPESampler.TPESamplerdirectly.This gives a safe default that prefers GP when possible but never hard-fails the run.
Why this helps
MotherML tuning commonly has:
GPSampleris often effective on the stable shared search space in this regime. But GP does not fully cover dynamic/conditional spaces. To avoid low-quality fallback behavior there, this PR configures a hybrid:Why GP + TPE together (and not GP alone)
GPSampleris strongest on a stable relative search space. In real MotherML tuning, many models (especially CatBoost-style spaces) include branching/conditional parameters.For those parameters, GP does not always provide relative/joint suggestions. Optuna then delegates to the sampler's
independent_sampler.If that independent sampler is random, quality can drop on expensive low-budget runs. Using TPE here gives a history-informed fallback after startup instead of staying purely random.
What changed in code
File:
src/mother/optimization/core.py_build_tpe_sampler(...)_build_gp_sampler(...)MotherTuner.__init__now:GPSampleris configured with:independent_sampler=optuna.samplers.TPESampler(...)n_startup_trialsvalue as GPImportant behavior (reviewer-friendly)
This is the key runtime behavior for startup and conditional parameters:
Detailed runtime flow for dynamic/conditional spaces
For each trial, sampling effectively works like this:
Important reviewer note: startup cutoff is study-level trial count, not "how many times a specific conditional parameter appeared."
In short:
What this PR does not change
To keep scope focused, this PR is about default sampler strategy and safe fallback behavior.
It does not change broader optimization workflow semantics beyond sampler choice and GP/TPE wiring.
It also does not claim full joint GP modeling over all conditional branches. This is a pragmatic hybrid to improve defaults in mixed spaces.
Test coverage
File:
test/unit/test_model_tuner.pyCovered in
TestDefaultSamplerSelection:Also validated:
TestGetCallbacksTrade-off summary
Pros:
Caveat:
Suggested PR wording
This PR makes
GPSamplerthe default Optuna sampler in MotherML when torch is available, withTPESampleras the independent sampler and as full fallback when GP cannot be initialized. The hybrid keeps GP benefits for low-budget optimization on stable shared spaces, while using history-informed TPE for startup and conditional parameters that fall outside GP's relative search space.