refactor(sampler): consolidate link neighbor sampling interface, part 2 #5365
Conversation
This file contains 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
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…ric into remote_backend_2
Codecov Report
@@ Coverage Diff @@
## master #5365 +/- ##
==========================================
+ Coverage 83.35% 83.39% +0.04%
==========================================
Files 342 342
Lines 18771 18771
==========================================
+ Hits 15646 15654 +8
+ Misses 3125 3117 -8
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
for more information, see https://pre-commit.ci
wsad1
reviewed
Sep 8, 2022
|
Didn't get around to reviewing before merged.. but all looks great to me :-) |
mananshah99
added a commit
that referenced
this pull request
Sep 12, 2022
This PR continues the effort to consolidate PyG's sampling interface in preparation for moving `sample(...)` behind the `GraphStore` interface. This effort is somewhat large in scope and will be broken into multiple PRs for ease of review. It resolves some TODOs from #5365. The major change removes special handling of `input_type` and `perm` / `perm_dict` in the neighbor loader. This enables a full separation between the sampler interface and the NeighborLoader (the only other interaction that is not clearly documented or exposed well in the sampler class is `SamplerOutput.metadata`, which will take a bit more work to properly define.
14 tasks
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.
This PR continues the effort to consolidate PyG's sampling interface in preparation for moving
sample(...)behind theGraphStoreinterface. This effort is somewhat large in scope and will be broken into multiple PRs for ease of review. It resolves some TODOs from #5312, and introduces others that will be resolved in follow-up PRs.The major change removes
LinkNeighborSampler, consolidating it under a the commonBaseSamplerinterface withNeighborSampler. In doing so, it modifies theBaseSamplerinterface to supportsample_from_nodesandsample_from_edges, since oftentimes edge-based sampling applies some transformations and then re-uses the same logic as node-based sampling. A sampler need not define both node-based and edge-based sampling, but if it doesn't, an appropriate error will be raised.Resolved TODOs:
LinkNeighborSamplerandNeighborSamplerinterfaces are alignededge_type_to_strinfilter_*perm_dict, which removes the need foredge_type_to_strentirely