Surface swallowed sampler-coroutine exceptions + warn on partial feature coverage#693
Draft
kmontemayor2-sc wants to merge 5 commits into
Draft
Surface swallowed sampler-coroutine exceptions + warn on partial feature coverage#693kmontemayor2-sc wants to merge 5 commits into
kmontemayor2-sc wants to merge 5 commits into
Conversation
…arding Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…xceptions Wrap BaseDistNeighborSampler._send_adapter in try/except: on failure, log the full traceback and forward a one-time poison-pill SampleMessage through the sampling channel (channel mode) or re-raise (channel-less mode). Decode the pill at the top of each concrete loader _collate_fn (DistNeighborLoader, DistABLPLoader) so the training process raises a RuntimeError with the worker traceback instead of hanging forever. Initialize the one-time-send guard in a new __init__. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… error tests Add BaseDistLoader._warn_on_partial_feature_coverage (called from __init__): for a heterogeneous dataset whose feature store covers only a subset of its sampled-eligible node/edge types, emit a non-fatal warning. Reachability of a featureless type is a runtime, frontier-dependent property that construction cannot prove, so this never raises; the sampling-error poison pill is the actual fast-fail. Add loader-level tests through the real multiprocess producer + ShmChannel path, each under a bounded proc.join(timeout) so a pre-fix regression fails instead of wedging CI: a reachable partial-edge-feature dataset raises promptly (not hangs), and an unreachable featureless edge type still yields batches while warning at construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
/all_test |
Contributor
GiGL Automation@ 22:07:08UTC : 🔄 @ 23:43:45UTC : ✅ Workflow completed successfully. |
Contributor
GiGL Automation@ 22:07:10UTC : 🔄 @ 22:08:56UTC : ✅ Workflow completed successfully. |
Contributor
GiGL Automation@ 22:07:10UTC : 🔄 @ 22:17:47UTC : ✅ Workflow completed successfully. |
Contributor
GiGL Automation@ 22:07:11UTC : 🔄 @ 23:28:29UTC : ✅ Workflow completed successfully. |
Contributor
GiGL Automation@ 22:07:12UTC : 🔄 @ 22:11:45UTC : ❌ Workflow failed. |
Contributor
GiGL Automation@ 22:07:12UTC : 🔄 @ 23:30:50UTC : ✅ Workflow completed successfully. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…agation only Remove _warn_on_partial_feature_coverage and its test: a featureless message-passing type is a legitimate shape handled by treating its features as empty, not something to warn about at loader construction. This branch is now scoped to the general error-handling change — swallowed sampler-coroutine exceptions surface as a prompt RuntimeError with the worker traceback instead of hanging. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kmontemayor2-sc
force-pushed
the
fix-b-surface-sampler-exceptions
branch
from
5c66612 to
2555093
Compare
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
1 participant
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.
What & why
GLT's
ConcurrentEventLoopcatches every exception raised inside a samplingcoroutine and only logs
coroutine task failed: <str(e)>— no traceback, nomessage sent to the channel. When that happens the loader blocks forever on
channel.recv(), so a single sampler failure turns into a silent, indefinitehang.
This PR makes those failures loud and immediate. It does not fix the
underlying feature-coverage bug that triggered the incident (that's a
separate PR) — it's the general guardrail that converts any swallowed sampler
exception into a fast, actionable error.
Changes
gigl/distributed/utils/sampling_errors.py(new): shared wire codec —reserved key
#SAMPLING_ERROR,encode_sampling_error()(traceback → uint8tensor),
raise_if_sampling_error()(decodes and raisesRuntimeError). Onesource of truth so producer and consumer can't drift.
base_sampler.py:BaseDistNeighborSamplergains an__init__thatinitializes
_sampling_error_sent;_send_adapteris wrapped in try/except —on failure it logs the full traceback via
logger.exceptionand forwards aone-time poison-pill
SampleMessagein channel mode (re-raises inchannel-less mode). One pill per sampler instance, so a failure storm can't
block a bounded channel.
distributed_neighborloader.py/dist_ablp_neighborloader.py:raise_if_sampling_error(msg)at the top of each_collate_fn, so thetraining process raises on the next
next(loader)with the worker's realtraceback.
base_dist_loader.py:_warn_on_partial_feature_coverage(called from__init__) — non-fatal warning when a heterogeneous dataset has features ononly a subset of its node/edge types. Warning, never a raise: reachability of
a featureless type is a runtime property that can't be proven at construction.
Testing
sampling_errors_test.py(new): 5/5 — codec round-trip incl. non-ASCII andempty-string, writable-tensor, no-op-when-absent.
remote_channel_test.py: 6/6 — poison pill survives the graph-store receivepath and raises loader-side.
distributed_neighborloader_test.py: 25/25 (1 pre-existing GPU skip) — incl.two loader-level tests under bounded
proc.join(timeout=...).make type_check: clean.RuntimeError("A sampling worker failed... KeyError: ...") in ~30s insteadof hanging.