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

[BUG] Adds option to build.sh to build without cugraphops, updates docs #2904

Merged

Conversation

rlratzel
Copy link
Contributor

@rlratzel rlratzel commented Nov 9, 2022

closes #2902
closes #2892

Allows from-source users to build without cugraphops. This will disable the sampling algos that depend on cugraphops, but otherwise cugraph builds normally.

Tested by removing libcugraphops from my environment, ensured the build error described in #2892 was seen, built again with the new option and observed the build succeed.

@rlratzel rlratzel added bug Something isn't working non-breaking Non-breaking change labels Nov 9, 2022
@rlratzel rlratzel added this to the 22.12 milestone Nov 9, 2022
@rlratzel rlratzel requested a review from a team as a code owner November 9, 2022 23:46
@rlratzel rlratzel self-assigned this Nov 9, 2022
@rlratzel rlratzel added the DO NOT MERGE Hold off on merging; see PR for details label Nov 10, 2022
@rlratzel
Copy link
Contributor Author

I added the "do not merge" label since I'm seeing errors with a local build using this option without libcugraphops installed (I made a bad assumption above that if cmake completed configuration successfully that the build would work).

/Projects/cugraph/cpp/src/prims/per_v_random_select_transform_outgoing_e.cuh(631): error: name followed by "::" must be a class or namespace name
          detected during:
            instantiation of "std::tuple<std::optional<rmm::device_uvector<size_t>>, decltype((<expression>))> cugraph::per_v_random_select_transform_outgoing_e(const raft::handle_t &, const GraphViewType &, const VertexFrontierBucketType &, EdgeSrcValueInputWrapper, EdgeDstValueInputWrapper, EdgeOp, raft::random::RngState &, size_t, __nv_bool, std::optional<T>, __nv_bool) [with GraphViewType=cugraph::graph_view_t<int32_t, int32_t, float, false, false, void>, VertexFrontierBucketType=cugraph::key_bucket_t<int32_t, void, false, false>, EdgeSrcValueInputWrapper=cugraph::detail::edge_major_property_view_t<int32_t, const int32_t *>, EdgeDstValueInputWrapper=cugraph::detail::edge_minor_property_view_t<int32_t, const int32_t *>, EdgeOp=cugraph::detail::sample_edges_op_t<int32_t, float, int32_t>, T=thrust::tuple<int32_t, int32_t, float, int32_t, int32_t, thrust::null_type, thrust::null_type, thrust::null_type, thrust::null_type, thrust::null_type>]"
/Projects/cugraph/cpp/src/sampling/detail/sampling_utils_impl.cuh(186): here
            instantiation of "std::tuple<rmm::device_uvector<GraphViewType::vertex_type>, rmm::device_uvector<GraphViewType::vertex_type>, std::optional<rmm::device_uvector<GraphViewType::weight_type>>> cugraph::detail::sample_edges(const raft::handle_t &, const GraphViewType &, raft::random::RngState &, const rmm::device_uvector<GraphViewType::vertex_type> &, size_t, __nv_bool) [with GraphViewType=cugraph::graph_view_t<int32_t, int32_t, float, false, false, void>]"
/Projects/cugraph/cpp/src/sampling/detail/sampling_utils_sg.cu(124): here
...

line 631 in that file is:

             cugraph::ops::gnn::graph::INVALID_ID<edge_t>,

cc @ChuckHastings

@rlratzel rlratzel changed the title Adds option to build.sh to build without cugraphops, updates docs [BUG] Adds option to build.sh to build without cugraphops, updates docs Nov 10, 2022
@ChuckHastings ChuckHastings requested review from a team as code owners November 10, 2022 23:28
@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2022

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.12@4802b28). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head d98e3db differs from pull request most recent head 8e2f971. Consider uploading reports for the commit 8e2f971 to get more accurate results

Additional details and impacted files
@@               Coverage Diff               @@
##             branch-22.12    #2904   +/-   ##
===============================================
  Coverage                ?   60.72%           
===============================================
  Files                   ?      122           
  Lines                   ?     6880           
  Branches                ?        0           
===============================================
  Hits                    ?     4178           
  Misses                  ?     2702           
  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.
📢 Do you have feedback about the report comment? Let us know in this issue.

…-ops, updated build.sh to pass option to disable cugraph-ops.
@rlratzel rlratzel removed the DO NOT MERGE Hold off on merging; see PR for details label Nov 11, 2022
@BradReesWork
Copy link
Member

rerun tests

@rlratzel
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 85c447b into rapidsai:branch-22.12 Nov 15, 2022
@rlratzel rlratzel deleted the branch-22.12-cugraphops_options branch September 28, 2023 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change
Projects
None yet
4 participants