[Feature] Fixed sampler with limit on sampled nodes/edges in batch subgraph #6668

ayushnoori · 2023-12-04T02:40:32Z

Description

Subgraph sampler that sets an upper bound on the number of nodes included in each layer of the sampled subgraph. At each layer, the frontier is randomly subsampled. Rare node types can also be upsampled by taking the scaled square root of the sampling probabilities (best strategy TBD). The new FixedSampler performs node-wise neighbor sampling and returns the subgraph induced by all the sampled nodes.

The relevant issue is: #6623, thanks to @frozenbugs and @jermainewang for their input and review.

Checklist

Please feel free to remove inapplicable items for your PR.

The PR title starts with [$CATEGORY] (such as [NN], [Model], [Doc], [Feature]])
Code is well-documented
Related issue is referred in this PR
All changes have test coverage

Please note that unit tests for FixedSampler are not yet written.

Fix typo in ShaDowKHopSampler sample() function

Merge commit

Merge latest DGL changes

dgl-bot · 2023-12-04T02:40:59Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-12-04T02:41:11Z

Commit ID: 928fe21

Build ID: 1

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2023-12-28T20:21:25Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2023-12-28T20:21:37Z

Commit ID: fc5ce86

Build ID: 2

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

python/dgl/dataloading/fixed.py

ayushnoori · 2024-01-28T00:40:02Z

Thanks for the edits, @frozenbugs. Happy to address all the formatting errors. Do you see any issues with the function implementation itself?

dgl-bot · 2024-01-28T00:44:04Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-01-28T00:44:15Z

Commit ID: 4228344

Build ID: 3

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2024-01-28T01:17:18Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-01-28T01:17:30Z

Commit ID: 6f8acdb

Build ID: 4

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

frozenbugs · 2024-01-29T10:06:22Z

@dgl-bot

dgl-bot · 2024-01-29T10:08:36Z

Commit ID: 6f8acdb

Build ID: 5

Status: ❌ CI test failed in Stage [Lint Check].

Report path: link

Full logs path: link

frozenbugs · 2024-01-29T10:09:57Z

the code LGTM now, can you run lintrunner -a to fix the lint error? and can you add a unit test for your code.

The test case can be put here: tests/python/pytorch/dataloading, and you can use this test case as example: https://github.com/dmlc/dgl/blob/master/tests/python/pytorch/graphbolt/impl/test_in_subgraph_sampler.py

dgl-bot · 2024-02-09T20:29:06Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-09T20:29:18Z

Commit ID: d42b1ec2bdca14a28da11c3e5cbb5cab5f2aa768

Build ID: 6

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

frozenbugs · 2024-02-19T02:35:36Z

@dgl-bot

dgl-bot · 2024-02-19T02:37:45Z

Commit ID: cdc82adc722c688ee0abd1125069d970bb157f05

Build ID: 7

Status: ❌ CI test failed in Stage [Lint Check].

Report path: link

Full logs path: link

dgl-bot · 2024-07-06T05:35:07Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-07-06T05:35:19Z

Commit ID: c11b871

Build ID: 11

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

ayushnoori · 2024-07-06T05:37:17Z

Hmm, not sure why CI test failed. I already ran lintrunner -a, this is what I see:

> lintrunner -a
Warning: Could not find a lintrunner config at: '.lintrunner.private.toml'. Continuing without using configuration file.
WARNING: No previous init data found. If this is the first time you're running lintrunner, you should run `lintrunner init`.
ok No lint issues.
Successfully applied all patches.

> git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean

Have also merged changes so this PR is up-to-date.

mfbalin · 2024-07-06T05:47:59Z

There are 2 lint checkers, you passed the initial one but not the one in the CI that runs after that.

https://dgl-ci-result.s3.us-west-2.amazonaws.com/dgl/PR-6668/10/10/logs/logs_dir/tmp1ptbor3n.log

************* Module dgl.dataloading.capped_neighbor_sampler
python/dgl/dataloading/capped_neighbor_sampler.py:65:4: W0221: Parameters differ from overridden 'sample' method (arguments-differ)
python/dgl/dataloading/capped_neighbor_sampler.py:156:27: C0103: Variable name "node_IDs" doesn't conform to snake_case naming style (invalid-name)
python/dgl/dataloading/capped_neighbor_sampler.py:107:27: W0612: Unused variable 'rel_type' (unused-variable)
python/dgl/dataloading/capped_neighbor_sampler.py:107:37: W0612: Unused variable 'dst_type' (unused-variable)

dgl-bot · 2024-07-06T06:07:24Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-07-06T06:07:36Z

Commit ID: 161edd6

Build ID: 12

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

ayushnoori · 2024-07-06T06:30:00Z

Thanks, @mfbalin. Should be fixed now.

mfbalin · 2024-07-06T06:31:30Z

@dgl-bot

dgl-bot · 2024-07-06T06:33:33Z

Commit ID: 161edd6

Build ID: 13

Status: ❌ CI test failed in Stage [Lint Check].

Report path: link

Full logs path: link

ayushnoori · 2024-07-06T06:42:25Z

Looks like the error still is:

************* Module dgl.dataloading.capped_neighbor_sampler
python/dgl/dataloading/capped_neighbor_sampler.py:65:4: W0221: Parameters differ from overridden 'sample' method (arguments-differ)

The function signature in my code is:

def sample(self, g, indices, exclude_eids=None):

The sample() function signature of dgl.dataloading.base.Sampler is:

def sample(self, g, indices):

This is intentional, I added an optional exclude_eids argument. All other parameters should match. @mfbalin, may you please advise why this is a problem? Thanks!

mfbalin · 2024-07-08T00:36:08Z

@ayushnoori You can add # pylint: disable=arguments-differ on the same line to disable the warning if you really need it to be that way.

dgl-bot · 2024-07-08T00:42:36Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-07-08T00:42:48Z

Commit ID: 722cce1

Build ID: 14

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

ayushnoori · 2024-07-08T00:44:48Z

Thanks, @mfbalin – done.

mfbalin · 2024-07-08T01:27:17Z

@dgl-bot

dgl-bot · 2024-07-08T01:57:32Z

Commit ID: 722cce1

Build ID: 15

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

ayushnoori

Feedback from @frozenbugs should now be addressed.

python/dgl/dataloading/capped_neighbor_sampler.py

dgl-bot · 2024-07-09T06:00:25Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-07-09T06:00:37Z

Commit ID: c01e31b1337109632410539871c2ae927255db3f

Build ID: 16

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

frozenbugs · 2024-07-09T06:02:20Z

Hi @ayushnoori , I have this PR merged. And it can be used in nightly DGL wheel, and next DGL release.
But please be aware that the lagacy dataloading will be fully deprecated and removed as I mentioned before, so soon in the future the code will be removed together with the legacy dataloading. If you want to retain it in DGL, please follow up with a Graphbolt implementation.

ayushnoori · 2024-07-12T07:22:11Z

Understood, thanks so much, @frozenbugs!

…bgraph (dmlc#6668) Co-authored-by: Hongzhi (Steve), Chen <chenhongzhi.nkcs@gmail.com>

ayushnoori and others added 8 commits November 20, 2023 17:40

Fix typo in ShaDowKHopSampler

68bd455

Fix typo in ShaDowKHopSampler sample() function

Merge branch 'dmlc:master' into master

f90a429

Draft of fixed sampler

f16d04a

Merge branch 'master' of https://github.com/ayushnoori/dgl

a0abcc2

Merge commit

Merge branch 'dmlc:master' into master

380f115

Merge branch 'master' of https://github.com/dmlc/dgl

7e454e2

Merge latest DGL changes

Merge branch 'master' of https://github.com/ayushnoori/dgl

ecb0a83

Merge latest DGL changes

Fix documentation

928fe21

frozenbugs requested a review from Rhett-Ying December 4, 2023 05:35

Merge branch 'master' into master

fc5ce86

frozenbugs reviewed Jan 25, 2024

View reviewed changes

Merge branch 'dmlc:master' into master

4228344

Address feedback from @frozenbugs

6f8acdb

Fixed linting errors

deb9855

Merge branch 'master' into master

c11b871

Fix second linter errors

161edd6

Disable differing arguments warning

722cce1

ayushnoori commented Jul 8, 2024

View reviewed changes

frozenbugs reviewed Jul 9, 2024

View reviewed changes

python/dgl/dataloading/capped_neighbor_sampler.py Outdated Show resolved Hide resolved

Update python/dgl/dataloading/capped_neighbor_sampler.py

7b8e30f

frozenbugs approved these changes Jul 9, 2024

View reviewed changes

Merge branch 'master' into master

df22001

frozenbugs merged commit 025767c into dmlc:master Jul 9, 2024
1 check passed

lijialin03 pushed a commit to lijialin03/dgl that referenced this pull request Jan 6, 2025

[Feature] Fixed sampler with limit on sampled nodes/edges in batch su…

5b76c08

…bgraph (dmlc#6668) Co-authored-by: Hongzhi (Steve), Chen <chenhongzhi.nkcs@gmail.com>

[Feature] Fixed sampler with limit on sampled nodes/edges in batch subgraph #6668

[Feature] Fixed sampler with limit on sampled nodes/edges in batch subgraph #6668

Conversation

ayushnoori commented Dec 4, 2023

Description

Checklist

dgl-bot commented Dec 4, 2023

dgl-bot commented Dec 4, 2023

dgl-bot commented Dec 28, 2023

dgl-bot commented Dec 28, 2023

ayushnoori commented Jan 28, 2024

dgl-bot commented Jan 28, 2024

dgl-bot commented Jan 28, 2024

dgl-bot commented Jan 28, 2024

dgl-bot commented Jan 28, 2024

frozenbugs commented Jan 29, 2024

dgl-bot commented Jan 29, 2024

frozenbugs commented Jan 29, 2024 • edited Loading

dgl-bot commented Feb 9, 2024

dgl-bot commented Feb 9, 2024

frozenbugs commented Feb 19, 2024

dgl-bot commented Feb 19, 2024

dgl-bot commented Jul 6, 2024

dgl-bot commented Jul 6, 2024

ayushnoori commented Jul 6, 2024

mfbalin commented Jul 6, 2024 • edited Loading

dgl-bot commented Jul 6, 2024

dgl-bot commented Jul 6, 2024

ayushnoori commented Jul 6, 2024

mfbalin commented Jul 6, 2024

dgl-bot commented Jul 6, 2024

ayushnoori commented Jul 6, 2024 • edited Loading

mfbalin commented Jul 8, 2024 • edited Loading

dgl-bot commented Jul 8, 2024

dgl-bot commented Jul 8, 2024

ayushnoori commented Jul 8, 2024

mfbalin commented Jul 8, 2024

dgl-bot commented Jul 8, 2024

ayushnoori left a comment

Choose a reason for hiding this comment

dgl-bot commented Jul 9, 2024

dgl-bot commented Jul 9, 2024

frozenbugs commented Jul 9, 2024

ayushnoori commented Jul 12, 2024

frozenbugs commented Jan 29, 2024 •

edited

Loading

mfbalin commented Jul 6, 2024 •

edited

Loading

ayushnoori commented Jul 6, 2024 •

edited

Loading

mfbalin commented Jul 8, 2024 •

edited

Loading