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

Update decompress_to_edgelist to handle edge types #4397

Merged

Conversation

naimnv
Copy link
Contributor

@naimnv naimnv commented May 8, 2024

Update decompress_to_edgelist to handle edge types

@naimnv naimnv requested a review from a team as a code owner May 8, 2024 19:56
Copy link

copy-pr-bot bot commented May 8, 2024

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@naimnv naimnv self-assigned this May 8, 2024
@naimnv naimnv added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 8, 2024
@naimnv
Copy link
Contributor Author

naimnv commented May 8, 2024

/okay to test

Copy link
Contributor

@seunghwak seunghwak left a comment

Choose a reason for hiding this comment

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

LGTM but @jnke2016 needs a decompress_to_edgelist function that takes just a graph view object and an edge_property_device_view_t object of integer types (i.e. int32_t and int64_t for now, and in the future, we may need to support floating point types as well). Do you think it is better to implement this in this PR or address this in a separate PR?

@naimnv
Copy link
Contributor Author

naimnv commented May 8, 2024

LGTM but @jnke2016 needs a decompress_to_edgelist function that takes just a graph view object and an edge_property_device_view_t object of integer types (i.e. int32_t and int64_t for now, and in the future, we may need to support floating point types as well). Do you think it is better to implement this in this PR or address this in a separate PR?

He can just pass it instead of edge_id_view if edge_property_device_view_t object is of integer types (i.e. int32_t and int64_t
or pass it instead of edge_weights_view if edge_property_device_view_t object is of floating point types. And he needs it for triangle count tests, IIRC.

Otherwise we need an additional 72 explicit instantiations and I can address it in a separate PR.

@seunghwak
Copy link
Contributor

seunghwak commented May 9, 2024

Otherwise we need an additional 72 explicit instantiations and I can address it in a separate PR.

Why 72? I assume 3 (vertex_t & edge_t combinations) * 4 (store_transposed & multi_gpu) * 2 (Int32_t & int64_t) = 24. And eventually, we may remove 32 bit vertex ID and 64 bit edge ID support and may change store_transposed & multi_gpu as run-time parameters, so this will become more like 4.

@naimnv
Copy link
Contributor Author

naimnv commented May 10, 2024

/okay to test

@naimnv
Copy link
Contributor Author

naimnv commented May 12, 2024

/okay to test

@naimnv
Copy link
Contributor Author

naimnv commented May 13, 2024

/merge

@rapids-bot rapids-bot bot merged commit 79acec9 into rapidsai:branch-24.06 May 13, 2024
130 of 131 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuGraph improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants