Skip to content

fix(training): avoid duplicate packed mask transfers - #1405

Open
JESUSROYETH wants to merge 2 commits into
roboflow:developfrom
JESUSROYETH:fix/packed-segmentation-transfer-memory
Open

fix(training): avoid duplicate packed mask transfers#1405
JESUSROYETH wants to merge 2 commits into
roboflow:developfrom
JESUSROYETH:fix/packed-segmentation-transfer-memory

Conversation

@JESUSROYETH

Copy link
Copy Markdown
Contributor

Packed segmentation targets are currently copied to CUDA as complete concatenated fields and then cloned per sample. For masks, that temporarily keeps both the packed CUDA field and the materialised tensors alive.

Changes

  • Materialise packed sample views directly into independently owned tensors on the destination device.
  • Preserve the list-of-dicts mutation contract used by the unpacked path.
  • Add CPU ownership, pinned/non-blocking CUDA, transfer-hook, and CUDA peak-memory regression coverage (the last one uses an actual mask field and asserts on torch.cuda.max_memory_allocated(), not just values).
  • transfer_batch_to_device was PackedTargets.to()'s only caller; add direct coverage for it (same-device identity return, and a CUDA device change that keeps the batch packed) so it doesn't go untested now that it's unused internally.

Memory and throughput

Synthetic mask batches (not COCO-sourced; repro/cuda_peak_memory.py), pinned, three instance-count/resolution sizes. CUDA peak statistics are reset for each synchronized repetition; "extra" is peak minus the memory retained by the returned tensors.

Batch Mask bytes Previous extra CUDA peak Fixed extra CUDA peak Previous time, median [range] ms Fixed time, median [range] ms
small 393,216 393,728 bytes 0 bytes 0.1289 [0.1245, 0.1359] 0.0973 [0.0965, 0.0994]
medium 3,145,728 3,146,240 bytes 0 bytes 0.3061 [0.3014, 0.3216] 0.3141 [0.3092, 0.3513]
large 10,616,832 10,617,856 bytes 0 bytes 0.9153 [0.9019, 0.9547] 0.9183 [0.9053, 1.7515]

The previous path's extra CUDA peak tracks the mask field size almost exactly (ratio 1.000-1.001 across the three sizes). That matches the described mechanism — the packed CUDA field and the materialised per-sample clones stay alive at the same time, until the packed object goes out of scope. The fixed path's extra peak is zero at each size. Timings are noisy at this scale and are not sold as a speedup either way.

A real COCO segmentation val2017 check repeated the complete 5,000-image traversal in three independent processes (577 packed batches per run). The three summaries were identical: the previous temporary allocation was 18,915,328 bytes at the median, 32,188,416 at p95, and 44,048,384 maximum; the fixed path measured 0 bytes at each point. The previous peak follows the complete packed-field size (18,913,604 median, 43,467,692 maximum).

A PackedTargets batch routes through the same to_list() call for each field it carries. Detection- and keypoint-only batches were not profiled separately here — their fields are boxes, labels, and small metadata tensors, so the previous duplicate was already negligible for them.

Real COCO segmentation input-pipeline throughput, using batch size 8, 16 workers, 20 warmup batches, 200 timed batches, and three independent processes:

Path Median images/s Three-process range
Previous packed transfer 273.562 265.225-273.761
Direct packed materialisation 269.562 263.249-274.531
Explicitly unpacked 178.512 177.507-179.127

The two packed ranges overlap, so no speed difference is claimed between them. Direct materialisation retains a 1.51x median advantage over unpacked loading.

Validation

  • COCO segmentation val2017 target parity: 625 batches, 5,000 images, 36,335 instances, and 3,536,994,240 mask bytes; zero mismatches. Boxes are matched by IoU, with minimum IoU 1.0.
  • Touched CPU/CUDA battery: 226 passed, 2 skipped.
  • CI-marker CPU suite (AGENTS.md's command plus the marker exclusions CI itself adds): 4,421 passed, 77 skipped, 100 deselected. 3 pre-existing failures, unrelated to this diff and present on unmodified develop: stale doctests in docs/hooks/package_version.py and tests/export/test_onnx_notes.py.
  • Precommit: 19 hooks passed, including strict mypy.
  • TDD: the new tests fail on unmodified develop and pass with this change.
  • Full COCO segmentation val2017 metric parity: 5,000 images with RFDETRSegNano, resolution 312, batch size 16, 16 workers, and BF16. With identical bounded chunks, accumulated prediction/ground-truth states match exactly and the reported bbox/segmentation metrics are equal. Both arms report bbox mAP 0.48922181129455566 and segmentation mAP 0.40316349267959595.

An initial control with unequal chunk boundaries produced small metric deltas (about 2e-6). Matching the chunk boundaries removed the mismatches, so the cause was batch composition, not the packed transfer itself.

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86%. Comparing base (6674d85) to head (edc894c).

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #1405   +/-   ##
=======================================
+ Coverage       86%     86%   +1%     
=======================================
  Files          114     114           
  Lines        14880   14890   +10     
=======================================
+ Hits         12753   12845   +92     
+ Misses        2127    2045   -82     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Borda
Borda requested a balanced review from Copilot August 25, 2026 16:04

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

Optimizes packed segmentation-target transfers to avoid duplicate CUDA mask allocations while preserving mutation semantics.

Changes:

  • Adds direct per-sample device materialization via PackedTargets.to_list().
  • Updates training transfer logic and documentation.
  • Adds CPU, CUDA, ownership, and memory regression tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/rfdetr/utilities/tensors.py Implements direct destination materialization.
src/rfdetr/training/module_data.py Uses the optimized transfer path.
tests/utilities/test_tensors.py Adds ownership, CUDA, and memory tests.
tests/training/test_module_data.py Verifies transfer-hook routing.
docs/learn/train/training-parameters.md Documents the new transfer behavior.
CHANGELOG.md Records the memory fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +912 to +919
def peak_extra(materialise):
torch.cuda.synchronize()
torch.cuda.reset_peak_memory_stats()
out = materialise()
torch.cuda.synchronize()
extra = torch.cuda.max_memory_allocated() - torch.cuda.memory_allocated()
del out
return extra
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