Repository navigation
fix(ptl): guard segment replay drain on failed fit - #27
Merged
Merged
Conversation
`_SegmentHeadCapture.clear()` now resets the holder before draining the recorded streams, so a failing synchronize still leaves no recordings behind. `_SegmentHeadCaptureLifecycle.on_exception` reports that drain failure as a warning, so the trainer's own exception reaches the caller and its teardown runs; `on_fit_end` still raises it. `_holder` returns None for a module without a segmentation holder and every hook is then a no-op, as in `network_capture.py`. Guard test names drop filler words, and the stage-helper certificate test lists `proto_fusion` among the certified stages. WP: 193e Provenance: none (review remediation of replay lifecycle error handling; repository code only) Assumptions: none Gate: tests/ptl/test_segment_head_capture_guards.py::TestRecordingLifetime, tests/ptl/test_segment_head_capture.py::test_stage_helpers_and_shared_storage_are_not_certified --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Signed-off-by: Jirka Borovec <6035284+Borda@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Promoted warnings can still mask the original training exception and interrupt teardown.
1 open issue
What changed in this PR
This PR hardens segmentation replay cleanup after failed training while preserving drain errors at normal fit completion.
Changes:
- Reset capture state before draining streams and warn on failed-fit drain errors.
- Make lifecycle hooks no-ops for modules without a segmentation holder.
- Add lifecycle regression tests, simplify test names, and include
proto_fusionin certification coverage.
| File | Description |
|---|---|
| tests/ptl/test_segment_head_capture.py | Include proto_fusion in stage certification. |
| tests/ptl/test_segment_head_capture_guards.py | Cover drain failures and missing holders; simplify names. |
| src/lucid_yolo/ptl/segment_head_capture.py | Harden capture reset and lifecycle cleanup. |
Review effort: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jirka Borovec <6035284+Borda@users.noreply.github.com>
`_SegmentHeadCaptureLifecycle.on_exception` replaces the `try`/`except UserWarning: pass` around its drain-failure warning with `contextlib.suppress(UserWarning)`; a warning filter that turns the warning into an error still cannot replace the trainer's original exception. WP: 193e Provenance: none (behaviour-preserving restructuring; repository code only) Assumptions: none Gate: tests/ptl/test_segment_head_capture_guards.py::TestRecordingLifetime --- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Signed-off-by: Jirka Borovec <6035284+Borda@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

_SegmentHeadCapture.clear()now resets the holder before draining the recorded streams, so a failing synchronize still leaves no recordings behind._SegmentHeadCaptureLifecycle.on_exceptionreports that drain failure as a warning, so the trainer's own exception reaches the caller and its teardown runs;on_fit_endstill raises it._holderreturns None for a module without a segmentation holder and every hook is then a no-op, as innetwork_capture.py.Guard test names drop filler words, and the stage-helper certificate test lists
proto_fusionamong the certified stages.WP: 193e
Provenance: none (review remediation of replay lifecycle error handling; repository code only)
Assumptions: none
Gate: tests/ptl/test_segment_head_capture_guards.py::TestRecordingLifetime, tests/ptl/test_segment_head_capture.py::test_stage_helpers_and_shared_storage_are_not_certified