Remove positive/negative-edge zero-fanout workaround in k-hop sampler#676
Remove positive/negative-edge zero-fanout workaround in k-hop sampler#676kmontemayor2-sc wants to merge 2 commits into
Conversation
DistNeighborSampler now skips label (positive/negative supervision) edges directly in its k-hop loop, mirroring DistPPRNeighborSampler. As a result patch_fanout_for_sampling no longer injects zero fanout for label edges; it excludes them from num_neighbors instead, resolving the long-standing TODO at neighborloader.py. Behavior-preserving (zero fanout and skipping produce identical subgraphs) and covers both colocated and graph-store modes. - dist_neighbor_sampler.py: skip is_label_edge_type edges before indexing num_neighbors - neighborloader.py: patch_fanout_for_sampling excludes label edges; add empty-edges guard - base_dist_loader.py: fix stale create_sampling_config docstring - neighborloader_test.py: update expectations (labels excluded, not zeroed) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
/all_test |
GiGL Automation@ 18:43:02UTC : 🔄 @ 20:06:55UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 18:43:02UTC : 🔄 @ 18:52:20UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 18:43:04UTC : 🔄 @ 18:50:54UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 18:43:06UTC : 🔄 @ 20:04:25UTC : ❌ Workflow failed. |
GiGL Automation@ 18:43:06UTC : 🔄 @ 19:47:48UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 18:43:07UTC : 🔄 @ 18:45:01UTC : ✅ Workflow completed successfully. |
| message_passing_edge_types = [ | ||
| edge_type for edge_type in edge_types if not is_label_edge_type(edge_type) | ||
| ] |
There was a problem hiding this comment.
Is this just here in case a user still provides a fanout with the labeled edge type?
There was a problem hiding this comment.
Hmmm that's a good point actually - do you think we should allow passing in label edge types? Maybe user, like, content becomes a label edge type, but we also want to use it for some other task?
WDYT? I guess we can allow fanout here (with a warning?)
patch_fanout_for_sampling now raises ValueError if a caller passes any label (positive/negative supervision) edge type in num_neighbors — previously these were silently dropped. The existing guard that raises when there are no message-passing edge types to fan out around is retained. Fail fast on invalid fanout input rather than silently correcting it. - neighborloader.py: dict branch rejects caller-provided label edge types - base_dist_loader.py: update create_sampling_config docstring - neighborloader_test.py: label-edge and no-message-passing-edges cases now assert raises - dist_ablp_neighborloader_test.py: build num_neighbors over message-passing edges only Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3 participants
DistNeighborSampler now skips label (positive/negative supervision) edges directly in its k-hop loop, mirroring DistPPRNeighborSampler. As a result patch_fanout_for_sampling no longer injects zero fanout for label edges; it excludes them from num_neighbors instead, resolving the long-standing TODO at neighborloader.py. Behavior-preserving (zero fanout and skipping produce identical subgraphs) and covers both colocated and graph-store modes.
Scope of work done
Where is the documentation for this feature?: N/A
Did you add automated tests or write a test plan?
Updated Changelog.md? NO
Ready for code review?: NO