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.

Sign up for GitHub

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

[REVIEW] Subgraph extraction python bindings #246

Merged
merged 11 commits into from
May 3, 2019
Merged

[REVIEW] Subgraph extraction python bindings #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ec780fb
Added gdf_extract_subgraph_vertex_nvgraph method wrapping nvgraphExtr…
jwyles Feb 21, 2019
ebb956a
Added python bindings for subgraph extraction
jwyles Feb 21, 2019
19cd365
WIP added tests for subgraph extraction, debugging errors
jwyles Feb 26, 2019
7364d1f
Added missing test file
jwyles Feb 27, 2019
1758c49
Merge branch 'master' into subgraph
jwyles Apr 25, 2019
6412436
Updated subgraph extraction to use get_gdf_column_view instead of cre…
jwyles Apr 25, 2019
8aa8e92
fix flake8 style errors
ChuckHastings Apr 29, 2019
a8a4f64
update CHANGELOG
ChuckHastings Apr 29, 2019
624e5c8
merge latest from branch-0.7
ChuckHastings May 3, 2019
19b593d
clean up some python documentation
ChuckHastings May 3, 2019
9c89346
address comments from PR
ChuckHastings May 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- PR #210 Expose degree calculation kernel via python API
- PR #220 Added bindings for Nvgraph triangle counting
- PR #234 Added bindings for renumbering, modify renumbering to use RMM
- PR #246 Added bindings for subgraph extraction
- PR #250 Add local build script to mimic gpuCI

## Improvements
Expand Down
58 changes: 34 additions & 24 deletions cpp/include/nvgraph_gdf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
* @return Error code
*/
//gdf_error gdf_createGraph_nvgraph(nvgraphHandle_t nvg_handle,
// gdf_graph* gdf_G,
// nvgraphGraphDescr_t * nvgraph_G,
// bool use_transposed = false);
// gdf_graph* gdf_G,
// nvgraphGraphDescr_t * nvgraph_G,
// bool use_transposed = false);

/**
* Wrapper function for Nvgraph SSSP algorithm
Expand All @@ -62,13 +62,13 @@ gdf_error gdf_sssp_nvgraph(gdf_graph *gdf_G, const int *source_vert, gdf_column
* @return Error code
*/
gdf_error gdf_balancedCutClustering_nvgraph(gdf_graph* gdf_G,
const int num_clusters,
const int num_eigen_vects,
const float evs_tolerance,
const int evs_max_iter,
const float kmean_tolerance,
const int kmean_max_iter,
gdf_column* clustering);
const int num_clusters,
const int num_eigen_vects,
const float evs_tolerance,
const int evs_max_iter,
const float kmean_tolerance,
const int kmean_max_iter,
gdf_column* clustering);

/**
* Wrapper function for Nvgraph spectral modularity maximization algorithm
Expand All @@ -84,14 +84,14 @@ gdf_error gdf_balancedCutClustering_nvgraph(gdf_graph* gdf_G,
* @param eig_vects Pointer to a GDF column in which the resulting eigenvectors will be stored
* @return
*/
gdf_error gdf_spectralModularityMaximization_nvgraph( gdf_graph* gdf_G,
const int n_clusters,
const int n_eig_vects,
const float evs_tolerance,
const int evs_max_iter,
const float kmean_tolerance,
const int kmean_max_iter,
gdf_column* clustering);
gdf_error gdf_spectralModularityMaximization_nvgraph(gdf_graph* gdf_G,
const int n_clusters,
const int n_eig_vects,
const float evs_tolerance,
const int evs_max_iter,
const float kmean_tolerance,
const int kmean_max_iter,
gdf_column* clustering);

/**
* Wrapper function for Nvgraph clustering modularity metric
Expand All @@ -115,9 +115,9 @@ gdf_error gdf_AnalyzeClustering_modularity_nvgraph(gdf_graph* gdf_G,
* @return Error code
*/
gdf_error gdf_AnalyzeClustering_edge_cut_nvgraph(gdf_graph* gdf_G,
const int n_clusters,
gdf_column* clustering,
float* score);
const int n_clusters,
gdf_column* clustering,
float* score);

/**
* Wrapper function for Nvgraph clustering ratio cut metric
Expand All @@ -128,10 +128,20 @@ gdf_error gdf_AnalyzeClustering_edge_cut_nvgraph(gdf_graph* gdf_G,
* @return Error code
*/
gdf_error gdf_AnalyzeClustering_ratio_cut_nvgraph(gdf_graph* gdf_G,
const int n_clusters,
gdf_column* clustering,
float* score);
const int n_clusters,
gdf_column* clustering,
float* score);

/**
* Wrapper function for Nvgraph extract subgraph by vertices
* @param gdf_G Pointer to GDF graph object, this is the input graph
* @param vertices Pointer to GDF column object which contains the list of vertices to extract
* @param result Pointer to GDF graph object, this is the output must be a valid pointer
* @return Error code
*/
gdf_error gdf_extract_subgraph_vertex_nvgraph(gdf_graph* gdf_G,
Copy link
Contributor

Choose a reason for hiding this comment

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

So as I long as I understand based on comments,

gdf_graph* gdf_g and gdf_column* vertices are input parameters and gdf_graph* result is an output parameter. A function input parameter list having (input, output, input) is unorthodox. It seems like other functions in this file places input parameters before output parameters in the function input parameter lists, we should follow that convention. Also, if this function is not modifying gdf_G and vertices, we should enforce "const correctness".

So the input parameter should be
(const gdf_graph* gdf_G, const gdf_column* vertices, gdf_graph* result).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Changed the order.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't make this const gdf_graph* gdf_G. The graph can be changed if there is no CSR representation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Changing vertices to const makes sense. However, that would require changing the implementation in nvgraph. It seems to me like this is more than we want to address now. In fact, we probably should talk about how much we want to change nvgraph other than bug fixes.

gdf_column* vertices,
gdf_graph* result);
/**
* Wrapper function for Nvgraph triangle counting
* @param G Pointer to GDF graph object
Expand Down
Loading