Skip to content

#310 Record stop_reason=failed on warmup or iter failure - #311

Merged
jathavaan merged 1 commit into
mainfrom
bugfix/310-stop-reason-fixed-on-warmup-failure
May 23, 2026
Merged

#310 Record stop_reason=failed on warmup or iter failure#311
jathavaan merged 1 commit into
mainfrom
bugfix/310-stop-reason-fixed-on-warmup-failure

Conversation

@jathavaan

Copy link
Copy Markdown
Collaborator

Summary

  • When a benchmark fails during warmup, the @monitor decorator now records stop_reason=failed regardless of use_sequential_stopping. Previously the non-sequential branch (Databricks national-scale + DuckDB/PostGIS national-scale) fell through to stop_reason=fixed even when no timed iteration ran.
  • The fallback now applies only on the success path: failure paths always set failed.
  • Closes stop_reason=fixed reported when warmup raises in non-sequential @monitor branch #310.

Test plan

  • Trigger a warmup failure on a non-sequential benchmark (e.g. Databricks default-2-nodes-large); confirm benchmark_metadata.parquet records stop_reason=failed and achieved_iterations=0.
  • Run a healthy non-sequential benchmark (e.g. Databricks broadcast-4-nodes-large); confirm stop_reason=fixed and achieved_iterations=5.
  • Run a sequential benchmark to completion (PIP small); confirm stop_reason=precision still records correctly.
  • Force a timed-iter failure mid-run on a sequential benchmark; confirm stop_reason=failed and partial samples persist.

Copilot AI review requested due to automatic review settings May 23, 2026 06:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes incorrect stop_reason reporting in the @monitor benchmarking decorator when a benchmark fails during warmup or iteration execution—specifically ensuring non-sequential benchmarks no longer fall back to stop_reason=fixed on failure paths.

Changes:

  • Ensure stop_reason is set to StopReason.FAILED whenever failure is not None, independent of use_sequential_stopping.
  • Restrict the existing stop_reason fallback logic to the success path only (i.e., when no failure occurred).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jathavaan
jathavaan merged commit e54b1d6 into main May 23, 2026
31 checks passed
@jathavaan
jathavaan deleted the bugfix/310-stop-reason-fixed-on-warmup-failure branch May 23, 2026 10:36
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.

stop_reason=fixed reported when warmup raises in non-sequential @monitor branch

2 participants