Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

[FIX] Remove duplication of RAFT python bindings #1407

Closed
4 tasks
afender opened this issue Feb 12, 2021 · 0 comments · Fixed by #1436
Closed
4 tasks

[FIX] Remove duplication of RAFT python bindings #1407

afender opened this issue Feb 12, 2021 · 0 comments · Fixed by #1436
Assignees
@Iroy30
Labels
improvement Improvement / enhancement to an existing function
Milestone
0.19

Comments

@afender
Copy link
Member

afender commented Feb 12, 2021 •
edited
Loading

Raft cython/python features are implemented and deployed from RAFT (see https://github.com/rapidsai/raft/blob/branch-0.18/BUILD.md#pythoncython-integration)

cuGraph should not add or duplicate bindings as seen for the handle:

cugraph/python/cugraph/structure/graph_primtypes.pxd

Line 28 in 2743020

handle_t() except +

This defeats the purpose of RAFT and creates room for conflicts. Notice that for the handle, Raft provides a wrapper for 7 member function while cuGraph added only one.

Task list:

  • Get rid of the duplicate handle binding
  • Update files to import from raft : from cugraph.raft.common.handle cimport *
  • Check for other duplicate and update
  • Move generic features like move outside of the graph_prims.px* after checking this is not already in raft
@afender afender added the bug Something isn't working label Feb 12, 2021
@afender afender added this to the 0.19 milestone Feb 12, 2021
@afender afender changed the title [BUG] Remove duplication RAFT python bindings [BUG] Remove duplication of RAFT python bindings Feb 16, 2021
@BradReesWork BradReesWork changed the title [BUG] Remove duplication of RAFT python bindings [FIX] Remove duplication of RAFT python bindings Feb 17, 2021
@BradReesWork BradReesWork added the improvement Improvement / enhancement to an existing function label Feb 17, 2021
@BradReesWork BradReesWork removed the bug Something isn't working label Feb 17, 2021
@Iroy30 Iroy30 self-assigned this Mar 2, 2021
@afender afender mentioned this issue Mar 3, 2021
Merged
@Iroy30 Iroy30 mentioned this issue Mar 3, 2021
Merged
@rapids-bot rapids-bot bot closed this as completed in #1436 Mar 4, 2021
rapids-bot bot pushed a commit that referenced this issue Mar 4, 2021
@Iroy30
Remove raft handle duplication (#1436)
65f2f5b 
Closes #1407

Authors:
  - @Iroy30

Approvers:
  - Alex Fender (@afender)
  - Brad Rees (@BradReesWork)

URL: #1436
rapids-bot bot pushed a commit that referenced this issue Mar 5, 2021
@afender
Streams infra + support in egonet (#1435)
e525040 
- Stream synchronization behavior: switched to per-thread default stream instead of the legacy default stream
- Update raft tag
- EgoNet upgrade to use `uvector` instead of `device_vector`
- EgoNet upgrade to execute on a different stream for each seed
- Perf analysis timers/app for EgoNet

Concurrency is limited by the number of available blocks on the device. Thrust-based codes may request a lot of blocks without a way to control this. In practice, smaller graphs leverage concurrency better than larger ones where tasks may end up waiting for available resources.

We may wait on #1407 before reconciling and merging this
Close #957

Authors:
  - Alex Fender (@afender)

Approvers:
  - Seunghwa Kang (@seunghwak)
  - Andrei Schaffer (@aschaffer)

URL: #1435
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@Iroy30 Iroy30

Labels
improvement Improvement / enhancement to an existing function
Projects
None yet
Milestone
0.19
Development

Successfully merging a pull request may close this issue.

Remove raft handle duplication Iroy30/cugraph
3 participants
@BradReesWork @Iroy30 @afender