Skip to content

fix(ci): enable ft-long fault-tolerance soak tests#1653

Open
guapisolo wants to merge 5 commits into
mainfrom
ci/enable-ft-long
Open

fix(ci): enable ft-long fault-tolerance soak tests#1653
guapisolo wants to merge 5 commits into
mainfrom
ci/enable-ft-long

Conversation

@guapisolo

@guapisolo guapisolo commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Enable all three ft-long H200 soak tests and stop GSM8K once its accuracy target passes.

Symptom & Reproduction

  • Symptom: The run-ci-ft-long path could not complete its three intended H200 soak tests: registrations were disabled, then enabled runs exceeded stale time budgets.
  • Reproduction: PR run 29293029757 reproduces the runtime timeouts after enabling the three previously disabled registrations.
PYTHONPATH=. python tests/ci/run_suite.py --hw cuda --suite stage-c-8-gpu-h200 --list-only --labels run-ci-ft-long

Root Cause

  1. register_cuda_ci stored a disabled reason for each ft-long registration.
  2. filter_tests skipped every matching registration while disabled remained set.
  3. est_time values predated the enabled H200 runs and produced 3000s and 11250s timeouts.
  4. MetricChecker.on_eval recorded target success without returning it to the training loop.

Fix

Remove the temporary registration gates and calibrate all three est_time values from observed H200 runtimes. Add the opt-in --ci-metric-checker-stop-on-success path so MetricChecker.on_eval reports cumulative success through RolloutManager.eval to train.py and train_async.py; the realistic GSM8K soak enables it while retaining the 0.55 threshold and 250-rollout upper bound.

Verification

  • Metric success test: test_on_eval_reports_cumulative_success covers cumulative success across evaluations.
  • Metric failure test: test_dispose_rejects_runs_without_a_successful_eval covers rejection when no evaluation reaches the target.
  • Rollout manager test: TestEval.test_reports_stop_after_metric_success covers the stop signal returned after a successful evaluation.
  • Argument test: test_ci_metric_checker_stop_on_success_flag_is_parsed covers the new opt-in CLI flag.
  • H200 soak run: 29355800844 passes all three enabled ft-long tests.
  • Early-stop signal: The GSM8K soak stops at rollout 139 after eval/gsm8k=0.561789 exceeds 0.55.

Review Focus

  • Scrutinize the stop signal across MetricChecker.on_eval, RolloutManager.eval, train.py, and train_async.py.
  • Scrutinize the opt-in boundary from miles_validate_args to scenario_realistic_gsm8k.py.
  • Scrutinize the three est_time calibrations relative to observed H200 elapsed times under the runner timeout multiplier.

The run-ci-ft-long label already reaches the full-node H200 suite, but all three matching registrations are disabled. Remove those temporary gates so the label schedules every FT soak entry.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request re-enables several fault-tolerance (FT) soak tests by removing the disabled parameter from the register_cuda_ci registration calls in test_trainer_ft_random_dp2_cp2_real_rollout.py, test_trainer_ft_random_dp2_cp2_tp2_ep2.py, and test_trainer_ft_realistic_gsm8k.py. I have no additional feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

The newly enabled ft-long tests exceeded their stale per-file budgets while still making progress. Propagate metric-checker success to the train loops, opt the realistic GSM8K soak into early completion without relaxing its threshold, and calibrate all three soak estimates from the observed H200 run.
guapisolo and others added 2 commits July 15, 2026 00:25
The standalone metric checker test is implicitly collected by stage-a-cpu and is not useful for this FT soak change. Drop it from the PR without changing the metric checker implementation or the remaining stop-on-success coverage.
@guapisolo guapisolo closed this Jul 15, 2026
@guapisolo guapisolo reopened this Jul 15, 2026
FT soak tests must complete their configured rollout counts instead of stopping after a metric passes. Remove the early-stop flag and its wiring, restore the original estimates, and keep realistic GSM8K disabled so this PR only enables the two random FT cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant