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

[Sparse] Add matrix slicing python API #6184

Merged
merged 19 commits into from
Aug 30, 2023

Conversation

xiangyuzhi
Copy link
Collaborator

Description

Define the python interfaces for matrix slicing operators, includes: rowwise_select(ids), columnwise_select(ids), rowwise_select(start, end), columnwise_select(start, end)

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
  • If the PR is for a new model/paper, I've updated the example index here.

Changes

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 21, 2023

To trigger regression tests:

  • @dgl-bot run [instance-type] [which tests] [compare-with-branch];
    For example: @dgl-bot run g4dn.4xlarge all dmlc/master or @dgl-bot run c5.9xlarge kernel,api dmlc/master

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 21, 2023

Commit ID: a45c0dbafd965333eabd6022519440e219161004

Build ID: 1

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

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 21, 2023

Commit ID: 304e003

Build ID: 2

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

Report path: link

Full logs path: link

@frozenbugs frozenbugs requested a review from czkkkkkk August 21, 2023 09:32
Copy link
Collaborator

@czkkkkkk czkkkkkk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest you to try Github copilot to improve your wording and coding efficiency.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 23, 2023

Commit ID: e3ede522cd3869d0b48fedb7f8567ebae42b2522

Build ID: 3

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

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 25, 2023

Commit ID: abf1d3f6f6f3eafc0f5d2a4ea19f1310b3d87944

Build ID: 4

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

Report path: link

Full logs path: link

@xiangyuzhi
Copy link
Collaborator Author

Now we have two API designs, one is row-column-select API, and another is index-range-select API.
row-column-select API:

def rowwise_select(index or range) # User decides whether index or range by input.
def columnwise_select(index or range)

index-range-select API:

def Index_select(dim, index) # User decides whether row or column by 'dim'.
def Range_select(dim, range) 

Or even we can combine them all like:

def select(dim, index or range) # User decides dim and index/range.

@frozenbugs do you have some comments?

@frozenbugs
Copy link
Collaborator

Now we have two API designs, one is row-column-select API, and another is index-range-select API. row-column-select API:

def rowwise_select(index or range) # User decides whether index or range by input.
def columnwise_select(index or range)

index-range-select API:

def Index_select(dim, index) # User decides whether row or column by 'dim'.
def Range_select(dim, range) 

Or even we can combine them all like:

def select(dim, index or range) # User decides dim and index/range.

@frozenbugs do you have some comments?

The index/range should just be tensor, right? And it is quite common to use dim in dgl Sparse API to indicate row wise and column wise, see (https://docs.dgl.ai/en/1.1.x/api/python/dgl.sparse_v0.html), so def select(dim, Tensor) should be good.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 28, 2023

Commit ID: a029282ec203c1d39fd8d3e044db5ee2d667c389

Build ID: 5

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

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 28, 2023

Commit ID: 4439af596b868457176102442e7fe881b2e01856

Build ID: 6

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

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 29, 2023

Commit ID: 592dd17a03e2247a31d1cb96eadd9cc6d5d659ac

Build ID: 7

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

Report path: link

Full logs path: link

xiangyuzhi and others added 2 commits August 29, 2023 07:05
Co-authored-by: Hongzhi (Steve), Chen <chenhongzhi.nkcs@gmail.com>
@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 29, 2023

Commit ID: 588bc332b931eb15fc490ae2c67de5e00cf41673

Build ID: 12

Status: ⚪️ CI test cancelled due to overrun.

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 29, 2023

Commit ID: 0c4c25845f19ea894c5bb434519431181d6f2d11

Build ID: 13

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

Report path: link

Full logs path: link

@xiangyuzhi
Copy link
Collaborator Author

@frozenbugs CI told me "Unused argument 'dim' (unused-argument)", I wonder how to pass it without changing API.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 29, 2023

Commit ID: bc5991e738d789643d11925b24f758fce50d8d17

Build ID: 14

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

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 29, 2023

Commit ID: 45742eeba330eadda5ad01c6b48c939d77e939e2

Build ID: 15

Status: ❌ CI test failed in Stage [Torch CPU Unit test].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 29, 2023

Commit ID: e088d53a31c935fbd4a044ff091f9cc8a7d9ee3c

Build ID: 16

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

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 29, 2023

Commit ID: 7259d1b9c2299582b8f1679cc9165ad324b94b77

Build ID: 17

Status: ❌ CI test failed in Stage [Torch CPU Unit test].

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 30, 2023

Commit ID: 8d16ec0f142df0a114d4a492513c8800e5344cea

Build ID: 18

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 30, 2023

Commit ID: 75b018c9fca8daae8dc8500924c8e4b0fcfd74f3

Build ID: 19

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@xiangyuzhi xiangyuzhi requested a review from frozenbugs August 30, 2023 03:07
Copy link
Collaborator

@czkkkkkk czkkkkkk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@dgl-bot
Copy link
Collaborator

dgl-bot commented Aug 30, 2023

Commit ID: 099c30b3cc8cce2c2a5a1cb16b5a5b81223ca47b

Build ID: 21

Status: ✅ CI test succeeded.

Report path: link

Full logs path: link

@xiangyuzhi xiangyuzhi merged commit ec8225d into dmlc:master Aug 30, 2023
@xiangyuzhi xiangyuzhi deleted the slicing-python branch September 13, 2023 08:03
peizhou001 pushed a commit to peizhou001/dgl that referenced this pull request Nov 27, 2023
Co-authored-by: Hongzhi (Steve), Chen <chenhongzhi.nkcs@gmail.com>
DominikaJedynak pushed a commit to DominikaJedynak/dgl that referenced this pull request Mar 12, 2024
Co-authored-by: Hongzhi (Steve), Chen <chenhongzhi.nkcs@gmail.com>
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.

4 participants