Skip to content

Flow generator saves its WORST epoch as the 'best' checkpoint (key_metric='SSIM' with direction inferred from the name) #182

Closed

Description

@DMZ0

Summary

The flow generator's ModelCheckpoint is configured to maximize a metric that is actually loss-like, so the checkpoint saved as "best" is the worst epoch of the run. Since projects.get_weightfile_from_rundir() prefers the "best" checkpoint, flow_generator.weights=latest then hands that worst flow generator to feature-extractor training.

Mechanism

deepethogram/base.py:359-361 infers the optimization direction from the metric's name:

key_metric = lightning_module.metrics.key_metric
mode = "min" if "loss" in key_metric else "max"
monitor = f"val/{key_metric}"

deepethogram/flow_generator/train.py:373 sets:

key_metric = "SSIM"

"SSIM" contains no "loss" substring, so mode="max". But the SSIM written into opticalflow_metrics.h5 is a loss-like quantity where lower is better — it correlates positively with val/loss.

Evidence

From a 21-epoch flow generator run (TinyMotionNet, preset=deg_f):

  • val/loss, val/L1, val/SSIM and val/smoothness all have argmin = epoch 17 and argmax = epoch 2
  • corr(val/SSIM, val/loss) = +0.64 (they move together, confirming lower-is-better)
  • lightning_checkpoints/ contains exactly epoch=2-step=3000.ckpt and last.ckpt
Epoch val/SSIM val/loss
2 0.00209 0.01061 saved as "best"
17 0.00177 0.00921 actually best
20 (last) 0.00184 0.00934

The checkpoint labelled "best" has a 13.6% worse validation loss than the final epoch, and is the worst epoch of the entire run on every reconstruction metric.

Impact

projects.get_weightfile_from_rundir() returns best in preference to last and checkpoint.pt. So the documented path

flow_generator.weights=latest

silently selects the worst flow generator produced by training, and every downstream feature extractor is initialized from it.

Note the classification stages are unaffected: their key_metric is f1_class_mean_nobg, which should indeed be maximized, so the name-based heuristic happens to give the right answer there.

Suggested fix

Make the direction an explicit property of the metric rather than inferring it from a substring of its name — e.g. have the Metrics class carry a key_metric_mode ("min"/"max") that each subclass sets, and use that in base.py. The current heuristic fails silently for any minimized metric not named *loss*.

Version

master @ 1541eec (v0.4.0)

Activity

  1. DMZ0 commented on Sep 13, 2026

    @DMZ0
    Author

    Additional evidence that this is an inconsistency rather than an intentional choice: base.py already handles this correctly 10 lines into the same class, for the LR scheduler.

    # deepethogram/base.py:89  -- scheduler: treats SSIM as minimize (correct)
    if self.metrics.key_metric == "loss" or self.metrics.key_metric == "SSIM":
        self.scheduler_mode = "min"
    else:
        self.scheduler_mode = "max"
    # deepethogram/base.py:360 -- ModelCheckpoint: treats SSIM as maximize (bug)
    mode = "min" if "loss" in key_metric else "max"

    So within a single file the LR scheduler and the checkpoint callback disagree about whether SSIM should be minimized or maximized. The scheduler's explicit list is right; the checkpoint's substring test is what selects the worst epoch.

    This also suggests the cleanest fix is a single source of truth (a key_metric_mode carried by the Metrics object) used by both call sites, rather than patching the substring test.

  2. jbohnslav commented on Sep 16, 2026

    @jbohnslav
    Owner

    Fixed with #183, thanks for the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions