Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bugfix][MetaPath2Vec]Fix MetaPath2Vec sample function bug (Issue #5514) #7875

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pyslia7326
Copy link

@pyslia7326 pyslia7326 commented Mar 13, 2025

Description

In the MetaPath2Vec sample function, dgl.sampling.random_walk generates node ID -1 when there are no neighbors following the given meta-path. For example, a sampled path may look like:
[user_0, company_1, product_-1, company_-1, user_-1]
When mapping these local node IDs to global node IDs, the path becomes:
[10, 1, 9, 4, 14]
However, the sub-path [9, 14] does not actually exist in the graph, leading to incorrect edge relationships.

Fix

By filtering out -1 nodes generated by random_walk, the issue of non-existent paths is resolved. However, this introduces an edge case where certain nodes (e.g., nodes 13 and 14 in the example graph) have no valid neighbors for the first edge. In such cases, the path length is 1, meaning the sample function does not generate any valid node pairs.

To handle this case, the model's forward function is modified to return a loss of 0.0 when there are no valid positive samples. This prevents errors and ensures the backward pass remains unaffected.

metapath2vec_bug

Checklist

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [$CATEGORY] (such as [NN], [Model], [Doc], [Feature]])
  • I've leverage the tools to beautify the python and c++ code.
  • The PR is complete and small, read the Google eng practice (CL equals to PR) to understand more about small PR. In DGL, we consider PRs with less than 200 lines of core code change are small (example, test and documentation could be exempted).
  • All changes have test coverage
  • Code is well-documented
  • To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change
  • Related issue is referred in this PR

When a node has no neighbors according to the meta-path, the random
walk from `dgl.sampling` will generate a node with id -1. When
transforming the local node id to global node id, it incorrectly
maps to the last node with the same type, resulting in non-existent
paths. By filtering out invalid nodes (i.e., nodes with id -1) in
the random walk process, non-existent paths are avoided.

The new sample function introduces two edge cases:
1. Sampling with 0 pairs of nodes, which generates an empty tensor,
leading to a NaN loss in the model’s forward function. This is
handled by checking and returning a loss of 0.0 to prevent errors
when there are no valid positive samples. Returning a loss of 0.0
ensures that the forward pass is unaffected and prevents
unnecessary computation.
2. Sampling with only 1 pair of nodes, where the `neg_score`
calculation is modified to use `.squeeze(-1)` to correctly handle
the dimension.
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.

1 participant