Describe the bug
Umbrella issue: the optimize_hparams driver (deeptab/models/_mixins/hpo.py) has several compounding defects.
- A crashed trial can be reported as the best configuration. The exception path returns
best_val_loss * 100 (hpo.py:193). When val loss is negative — routine for LSS NLL — that is an extremely good objective value, so gp_minimize can select the crashing hyperparameters as result.x, which is what optimize_hparams returns and applies to the config.
- Pruning threshold inverts for negative losses.
early_pruning_threshold = best_epoch_val_loss * 1.5 (hpo.py:166-168): for a negative best loss the threshold is below the best, so every subsequent trial is pruned at prune_epoch (lightning_module.py:588), gutting the search.
head_layer_size_length corrupts head_layer_sizes in the final apply loop. The trial loop special-cases key == "head_layer_size_length" first (hpo.py:141), but the final best-params loop only checks key.startswith("head_layer_size_") (hpo.py:201-203) — which the length key matches. The length value (1–5) goes through round_to_nearest_16 → 0 and is prepended as a bogus layer size, and the list is never truncated to the optimized length: config.head_layer_sizes ends up like [0, s1, s2, s3, s4, s5] — a config no trial ever evaluated, with a zero-width layer. (The sibling layer_size_ branch is dead: get_search_space never emits those names.)
- Best hyperparameters are never refit. After
gp_minimize, only setattr(self.config, ...) happens — self._task_model still holds the last trial's weights and architecture, so model.predict() after optimize_hparams() uses an arbitrary trial while the config claims otherwise. Either refit with the best config or document that the user must call fit again.
- HPO ignores the estimator's seed:
gp_minimize(..., random_state=42) is hardcoded (hpo.py:195), so self.random_state cannot influence the search.
- Pruning epochs are off by one:
on_validation_epoch_end has no sanity_checking guard (lightning_module.py:580-590), so val_losses[0] is the 2-batch sanity-check loss of the untrained model (verified: a 2-epoch fit yields 3 entries), and epoch_val_loss_at(prune_epoch) — used for the pruning baseline — actually returns the loss after prune_epoch − 1.
To Reproduce
For (3), which is deterministic:
# "head_layer_size_length".startswith("head_layer_size_") is True,
# and round_to_nearest_16(1..5) == 0 -> [0, s1, ..., s5] applied to the config
For (1)/(2): run optimize_hparams on any LSS model whose baseline validation NLL is negative and make one trial raise; inspect the returned result.x / observe every trial pruned after prune_epoch.
Expected behavior
Failed trials should return a bad objective regardless of sign (e.g. abs(best) * 100 + large_const, or skopt's failure handling); the pruning threshold must be sign-aware; the final apply loop must mirror the trial loop's key handling; the model should be refit on the best config (or the API should document that it is not); the sanity-check validation must not enter val_losses.
Screenshots
n/a
Desktop (please complete the following information):
- OS: macOS (Darwin 25.5.0, arm64)
- Python version: 3.11.15
- deeptab Version: 2.0.0 (main @ 4e6a359)
Additional context
Item 6 verified by execution; the rest by code inspection (the sign-flip logic in 1–2 is unambiguous from the source). Since optimize_hparams mutates configs via setattr, note also that dataclass __post_init__ validation never re-runs on mutation, so an invalid trial config is only caught at model construction, if at all.
Describe the bug
Umbrella issue: the
optimize_hparamsdriver (deeptab/models/_mixins/hpo.py) has several compounding defects.best_val_loss * 100(hpo.py:193). When val loss is negative — routine for LSS NLL — that is an extremely good objective value, sogp_minimizecan select the crashing hyperparameters asresult.x, which is whatoptimize_hparamsreturns and applies to the config.early_pruning_threshold = best_epoch_val_loss * 1.5(hpo.py:166-168): for a negative best loss the threshold is below the best, so every subsequent trial is pruned atprune_epoch(lightning_module.py:588), gutting the search.head_layer_size_lengthcorruptshead_layer_sizesin the final apply loop. The trial loop special-caseskey == "head_layer_size_length"first (hpo.py:141), but the final best-params loop only checkskey.startswith("head_layer_size_")(hpo.py:201-203) — which the length key matches. The length value (1–5) goes throughround_to_nearest_16→ 0 and is prepended as a bogus layer size, and the list is never truncated to the optimized length:config.head_layer_sizesends up like[0, s1, s2, s3, s4, s5]— a config no trial ever evaluated, with a zero-width layer. (The siblinglayer_size_branch is dead:get_search_spacenever emits those names.)gp_minimize, onlysetattr(self.config, ...)happens —self._task_modelstill holds the last trial's weights and architecture, somodel.predict()afteroptimize_hparams()uses an arbitrary trial while the config claims otherwise. Either refit with the best config or document that the user must callfitagain.gp_minimize(..., random_state=42)is hardcoded (hpo.py:195), soself.random_statecannot influence the search.on_validation_epoch_endhas nosanity_checkingguard (lightning_module.py:580-590), soval_losses[0]is the 2-batch sanity-check loss of the untrained model (verified: a 2-epoch fit yields 3 entries), andepoch_val_loss_at(prune_epoch)— used for the pruning baseline — actually returns the loss afterprune_epoch − 1.To Reproduce
For (3), which is deterministic:
For (1)/(2): run
optimize_hparamson any LSS model whose baseline validation NLL is negative and make one trial raise; inspect the returnedresult.x/ observe every trial pruned afterprune_epoch.Expected behavior
Failed trials should return a bad objective regardless of sign (e.g.
abs(best) * 100 + large_const, or skopt's failure handling); the pruning threshold must be sign-aware; the final apply loop must mirror the trial loop's key handling; the model should be refit on the best config (or the API should document that it is not); the sanity-check validation must not enterval_losses.Screenshots
n/a
Desktop (please complete the following information):
Additional context
Item 6 verified by execution; the rest by code inspection (the sign-flip logic in 1–2 is unambiguous from the source). Since
optimize_hparamsmutates configs viasetattr, note also that dataclass__post_init__validation never re-runs on mutation, so an invalid trial config is only caught at model construction, if at all.