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

[ENH] Use RAFT error handling mechanism #951

Closed
seunghwak opened this issue Jun 16, 2020 · 5 comments · Fixed by #954
Closed

[ENH] Use RAFT error handling mechanism #951

seunghwak opened this issue Jun 16, 2020 · 5 comments · Fixed by #954
Assignees
Milestone

Comments

@seunghwak
Copy link
Contributor

Describe the solution you'd like
RAFT error handling macros are updated (rapidsai/raft#15). We need to use this instead of redefining our own (our CUGRAPH_EXPECTS and CUGRAPH_FAIL need to be updated to use the RAFT mechanism).

@seunghwak seunghwak added the ? - Needs Triage Need team to review and classify label Jun 16, 2020
@afender
Copy link
Member

afender commented Jun 16, 2020

So should we just use RAFT_EXPECTS/RAFT_FAIL until we have a use case for cugraph specific error macros?
We will just keep reconciling them otherwise.
IIRC the only difference is the "raft" vs "cugraph" string in the error message

@afender
Copy link
Member

afender commented Jun 16, 2020

Let's also upgrade the RAFT git tag in the cmake to get the latest RAFT version that contains rapidsai/raft#15

@afender
Copy link
Member

afender commented Jun 16, 2020

And there are many redefined warnings (CUDA_TRY, CUDA_CHECK, THROW). They need to be deleted from cuGraph

@afender afender added this to the 0.15 milestone Jun 16, 2020
@afender afender added 1 - On Deck and removed ? - Needs Triage Need team to review and classify labels Jun 16, 2020
@seunghwak
Copy link
Contributor Author

So should we just use RAFT_EXPECTS/RAFT_FAIL until we have a use case for cugraph specific error macros?
We will just keep reconciling them otherwise.
IIRC the only difference is the "raft" vs "cugraph" string in the error message

CUGRAPH_EXPECTS(FAIL) will throw cugraph::logic_error while RAFT_EXPECTS(FAIL) will throw raft::logic_error in addition to the difference in error message.

And cuDF uses CUDF_EXPECTS (cuML currently uses ASSERT but I guess they will switch to CUML_EXPECTS) and I think we'd better use CUGRAPH_EXPECTS(FAIL) and use this for now.

@seunghwak seunghwak self-assigned this Jun 18, 2020
@seunghwak
Copy link
Contributor Author

I assigned this to myself. This will be a quick fix.

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 a pull request may close this issue.

2 participants