-
Notifications
You must be signed in to change notification settings - Fork 304
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
Avoid directly calling graph constructor (as code cleanup before edge property support in primitives) #2834
Avoid directly calling graph constructor (as code cleanup before edge property support in primitives) #2834
Conversation
cpp/include/cugraph/graph.hpp
Outdated
@@ -28,10 +28,6 @@ | |||
#include <type_traits> | |||
#include <vector> | |||
|
|||
/** @defgroup cpp_api cuGraph C++ API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a documentation mechanism for doxygen. Should probably put this back.
It allows all of the things inside this comment group to be linked on a single documentation page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
cpp/include/cugraph/graph.hpp
Outdated
@@ -556,7 +533,3 @@ __host__ __device__ std::enable_if_t<std::is_unsigned<vertex_t>::value, bool> is | |||
} // namespace cugraph | |||
|
|||
#include "eidecl_graph.hpp" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bottom of the documentation group
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
cpp/include/cugraph/graph.hpp
Outdated
@@ -483,25 +479,6 @@ class graph_t<vertex_t, edge_t, weight_t, store_transposed, multi_gpu, std::enab | |||
private: | |||
friend class cugraph::serializer::serializer_t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also get rid of serializer_t
? Seems like with graph_bcast.hpp
gone this class is unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## branch-22.12 #2834 +/- ##
===============================================
Coverage ? 60.40%
===============================================
Files ? 114
Lines ? 6551
Branches ? 0
===============================================
Hits ? 3957
Misses ? 2594
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@gpucibot merge |
Should unblock #2834 This removes all of the algorithms from `cython.cu`. Closes #2498 Marked as breaking since we remove some old python bindings, they should no longer be used. Authors: - Chuck Hastings (https://github.com/ChuckHastings) Approvers: - Seunghwa Kang (https://github.com/seunghwak) - Joseph Nke (https://github.com/jnke2016) - Brad Rees (https://github.com/BradReesWork) URL: #2955
Delete dead code and avoid directly calling graph_t constructor (use create_graph_from_edgelist instead) as a pre-requsite for removing edge weights from graph_t class and edge property support in graph primitives.
bool renumber
is added tocoarsen_graph
to support MG Louvain testing (instead of replicating all the code related graph coarsening to support graph coarsening without renumbering).This PR also removed GRAPH_TEST as we don't expect users to directly call graph_t constructors.
Breaking as
bool renumber
is added to the public functioncoarsen_graph