Skip to content

Commit f30f3bc

Browse files
Felipedinoclaude
andcommitted
Build the pruning tests' objective from the unit that fits
Two optimizer test files built the objective they measure by reaching for HoldoutEvaluationStrategy.evaluate. That method is gone: the strategies declare how a run is evaluated and the fitting unit carries it out, so the objective comes from there now. The tests themselves are unchanged -- they still check that a bad trial is pruned, that disabling the pruner completes every trial, and that a real model reports each epoch to its trial. Which partitions a trial records is still read off the strategy class, the same way the job reads it, so the declaration stays connected to what it produces. These four failures were not caught earlier because the verification runs had been narrowed to the directories this work was touching -- units, dag, spike, api and evaluation -- after the full suite was dropped for containing a test that builds the app against the real ~/.DashAI. Deselecting that one test was the right answer; shrinking the suite to what seemed relevant was not, and it is precisely the change that removes a caller elsewhere that this hides. Whole suite: 3682 passed, one test deselected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d476cf0 commit f30f3bc

2 files changed

Lines changed: 44 additions & 16 deletions

File tree

tests/back/optimizers/test_optuna_pruning_integration.py

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,18 +29,32 @@
2929
from DashAI.back.evaluation.holdout import HoldoutEvaluationStrategy
3030
from DashAI.back.models.base_model import BaseModel
3131
from DashAI.back.optimizers.optuna_optimizer import OptunaOptimizer
32+
from DashAI.back.units.fit_model_unit import FitModelUnit
3233

3334

3435
def _holdout_evaluate(model, input_dataset, output_dataset, metric):
35-
"""The real holdout evaluation path, on a strategy with no factory.
36-
37-
`evaluate` reads which partitions its strategy scores, so it needs a real
38-
instance rather than None for self. Building one through __init__ would
39-
need a `ModelFactory` this test does not have, and does not need: the only
40-
thing read off the instance is a class attribute.
36+
"""The real objective of a holdout search: one fit, then score validation.
37+
38+
It used to be ``HoldoutEvaluationStrategy.evaluate``. The strategies now
39+
only declare how a run is evaluated, and the unit that fits carries it out,
40+
so the objective the optimizer measures comes from there. Which partitions
41+
a trial records is still the strategy's declaration -- the job reads
42+
SCORED_SPLITS off it and hands it over, minus the test partition, which a
43+
trial may never score -- so it is read off the class here too.
4144
"""
42-
strategy = HoldoutEvaluationStrategy.__new__(HoldoutEvaluationStrategy)
43-
return strategy.evaluate(model, input_dataset, output_dataset, metric)
45+
trial_splits = [
46+
split.name
47+
for split in HoldoutEvaluationStrategy.SCORED_SPLITS
48+
if split.name != "TEST"
49+
]
50+
unit = FitModelUnit(
51+
optimizer={"component": "", "params": {}},
52+
goal_metric="",
53+
run_id=None,
54+
artifact_prefix=None,
55+
trial_splits=trial_splits,
56+
)
57+
return unit._score_one_trial(model, input_dataset, output_dataset, metric)
4458

4559

4660
EPOCHS = 12

tests/back/optimizers/test_optuna_real_model.py

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,32 @@
4040
from DashAI.back.metrics.classification.accuracy import Accuracy
4141
from DashAI.back.models.mlp_image_classifier import MLPImageClassifier
4242
from DashAI.back.optimizers.optuna_optimizer import OptunaOptimizer
43+
from DashAI.back.units.fit_model_unit import FitModelUnit
4344

4445

4546
def _holdout_evaluate(model, input_dataset, output_dataset, metric):
46-
"""The real holdout evaluation path, on a strategy with no factory.
47-
48-
`evaluate` reads which partitions its strategy scores, so it needs a real
49-
instance rather than None for self. Building one through __init__ would
50-
need a `ModelFactory` this test does not have, and does not need: the only
51-
thing read off the instance is a class attribute.
47+
"""The real objective of a holdout search: one fit, then score validation.
48+
49+
It used to be ``HoldoutEvaluationStrategy.evaluate``. The strategies now
50+
only declare how a run is evaluated, and the unit that fits carries it out,
51+
so the objective the optimizer measures comes from there. Which partitions
52+
a trial records is still the strategy's declaration -- the job reads
53+
SCORED_SPLITS off it and hands it over, minus the test partition, which a
54+
trial may never score -- so it is read off the class here too.
5255
"""
53-
strategy = HoldoutEvaluationStrategy.__new__(HoldoutEvaluationStrategy)
54-
return strategy.evaluate(model, input_dataset, output_dataset, metric)
56+
trial_splits = [
57+
split.name
58+
for split in HoldoutEvaluationStrategy.SCORED_SPLITS
59+
if split.name != "TEST"
60+
]
61+
unit = FitModelUnit(
62+
optimizer={"component": "", "params": {}},
63+
goal_metric="",
64+
run_id=None,
65+
artifact_prefix=None,
66+
trial_splits=trial_splits,
67+
)
68+
return unit._score_one_trial(model, input_dataset, output_dataset, metric)
5569

5670

5771
EPOCHS = 3

0 commit comments

Comments
 (0)