Skip to content

fix: resolve #1318 Verify task: prm on real CUDA hardware after #... - #1319

Closed
chenzeyan54-commits wants to merge 1 commit into
MakazhanAlpamys:mainfrom
chenzeyan54-commits:fix/issue-1318
Closed

chenzeyan54-commits wants to merge 1 commit into
MakazhanAlpamys:mainfrom
chenzeyan54-commits:fix/issue-1318

Conversation

@chenzeyan54-commits

Copy link
Copy Markdown

What changed

Fixes #1318

Overview

Follow-up to #1235, fixed by #1250 (0334b975). The fix is covered by CPU tests and one tiny CUDA run. The runs below were not done before #1235 clos...

Key Changes

Verification

  • Verified patch minimal footprint against target file.
  • Automated Cloud CI workflow will run verification upon submission.

…fter MakazhanAlpamys#1250: one real run on an Ampere-or-newer card and one on a pre-Ampere card (T4/V100)

Follow-up to MakazhanAlpamys#1235, fixed by MakazhanAlpamys#1250 (`0334b975`). The fix is covered by CPU tests and one tiny CUDA run. The runs below were not done before MakazhanAlpamys#1235 clos...

Signed-off-by: chenzeyan54-commits <chenzeyan54-commits@users.noreply.github.com>
@MakazhanAlpamys

Copy link
Copy Markdown
Owner

@chenzeyan54-commits, thank you for picking up #1318.

#1318 asks for something a pull request cannot supply: the output of real training runs on real GPUs. The two tests added here do not produce that:

  • GitHub's runners have no GPU, so both tests are skipped in CI (SKIPPED in the macOS and Windows logs). On a machine with a card they train the 90,497-parameter test model from the prm_base fixture, not a real base through soup train. That is the RTX 5070 run the issue already describes as done.
  • The fp16 test forces the pre-Ampere flags on whichever card runs it. The issue reports that exact configuration as inconclusive (the GradScaler skipped all 3 steps), and asks for a run on an actual pre-Ampere card (T4/V100) instead.
  • Fixes #1318 would close the issue on merge with none of its acceptance items done.

For reference, CI also fails on the diff itself. ruff reports 5 errors: unused torch imports at lines 588 and 617, lines over 100 characters at 595 and 624, and no newline at the end of the file. tests/test_issue833_gpu_marker.py::TestNoPrivateProbe::test_no_test_module_probes_cuda_itself fails because the new class calls torch.cuda.is_available() itself; tests that need CUDA use @pytest.mark.gpu, and @pytest.mark.cuda is not a registered marker. The branch also conflicts with main, where #1314 changed the end of the same file.

What resolves #1318 is a comment on the issue with:

  1. Ampere or newer (bf16): the PRM example from docs/training.md run through soup train on a small real base such as HuggingFaceTB/SmolLM2-360M-Instruct, with a prm-format file of a few hundred rows (the row shape is in soup train --resume on task: prm / moe_lora_routing says "Resuming from ...", then retrains from step 0 and overwrites the checkpoints #1220), batch_size: 1 and at least 20 optimizer steps. Report the card, driver and torch version; that no step raises; the loss curve; peak VRAM next to the pre-flight estimate of 16 bytes per parameter plus activations; and a short safetensors comparison showing the saved weights differ from the base's.
  2. Pre-Ampere (fp16 + GradScaler), for example a T4 on Colab or Kaggle: the same config, showing that no _amp_foreach_non_finite_check_and_unscale_cuda ... 'BFloat16' error appears, that the GradScaler takes optimizer steps once its warm-up back-off ends, and that the loss falls.
  3. Optionally, the same run with a ZeRO-2 DeepSpeed config on one GPU.

Each with the exact config, command and output. A run that fails is as useful as one that passes.

Since #1318 is answered by runs rather than by a change to the repository, I'm closing this PR. If you have access to a GPU, those runs posted on #1318 would be very welcome. For issues that do need code, the suggestions from #1201 still apply: claim the issue in a comment first, and run ruff check and the nearest test file before pushing.

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.

Verify task: prm on real CUDA hardware after #1250: one real run on an Ampere-or-newer card and one on a pre-Ampere card (T4/V100)

2 participants