Skip to content

Raise error if best_metrics.metrics.watch_metric != last_metrics.watch_metric#659

Merged
yutong-xiang-97 merged 4 commits intomainfrom
yutong-trn-1842-raise-error-if-best_metricsmetricswatch_metric
Mar 23, 2026
Merged

Raise error if best_metrics.metrics.watch_metric != last_metrics.watch_metric#659
yutong-xiang-97 merged 4 commits intomainfrom
yutong-trn-1842-raise-error-if-best_metricsmetricswatch_metric

Conversation

@yutong-xiang-97
Copy link
Copy Markdown
Contributor

What has changed and why?

As the title suggests.

How has it been tested?

An additional unit test.

Did you update CHANGELOG.md?

  • Yes
  • Not needed (internal change)

Did you update the documentation?

  • Yes
  • Not needed (internal change without effects for user)

Copilot AI review requested due to automatic review settings March 23, 2026 10:00
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens correctness in best-metric tracking by failing fast when the “best so far” metrics and the “latest” metrics disagree on which watch metric is being tracked, preventing silent mis-selection of best checkpoints.

Changes:

  • Change get_best_metrics to raise a RuntimeError when best.watch_metric != last.watch_metric instead of silently returning the previous best.
  • Add a unit test that asserts the new error is raised on mismatched watch metrics.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/lightly_train/_commands/train_task_helpers.py Raises an explicit error when best vs. last aggregated metrics use different watch_metric.
tests/_commands/test_train_task_helpers.py Adds a pytest unit test covering the new mismatch error behavior.

@yutong-xiang-97
Copy link
Copy Markdown
Contributor Author

/review

@yutong-xiang-97 yutong-xiang-97 enabled auto-merge (squash) March 23, 2026 10:28
@yutong-xiang-97 yutong-xiang-97 merged commit 87c507e into main Mar 23, 2026
16 checks passed
@yutong-xiang-97 yutong-xiang-97 deleted the yutong-trn-1842-raise-error-if-best_metricsmetricswatch_metric branch March 23, 2026 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants