cugraph icon indicating copy to clipboard operation
cugraph copied to clipboard

Fix building cugraph with CCCL main

Open trxcllnt opened this issue 1 year ago • 4 comments

Similar to https://github.com/rapidsai/cudf/pull/15552, we are testing building RAPIDS with CCCL's main branch to get ahead of any breaking changes.

trxcllnt avatar May 08 '24 21:05 trxcllnt

The new kv_store_t constructor is necessary because there are code paths that attempt to construct a thrust::tuple from a scalar (invalid_vertex_id<vertex_t>::value here).

While this is possible in CCCL 2.2.0's version of thrust::tuple, I checked with @jrhemstad it was definitely never intended to work. CCCL 2.4+ makes thrust::tuple an alias of cuda::std::tuple, so this behavior is now gone, and cuGraph needs to be updated.

I commented out the prims/mg_extract_transform_e.cu test for now, because the compiler resisted my attempts to rewrite this functor to be compatible with the new CCCL behavior.

The problem case is when the key_t template parameter is thrust::tuple<vertex_t, int32_t>, but the functor is called with the invalid value scalar -1.

Alternatively, I attempted to specialize the invalid_idx struct to return a thrust::tuple if the vertex_t template parameter is a tuple, but that seemed to break in other ways.

cc: @BradReesWork if you have any ideas or know the right person to help fix this.

trxcllnt avatar May 09 '24 19:05 trxcllnt

The nvcc error tells us half of the issue:

mg_extract_transform_e.cu(210): error: no instance of function template "cugraph::extract_transform_e" matches the argument list
              argument types are: (raft::handle_t, cugraph::graph_view_t<int32_t, int32_t, false, true, void>, cugraph::detail::edge_major_property_view_t<int32_t, const int32_t *, int32_t>, cugraph::detail::edge_minor_property_view_t<int32_t, const int32_t *, int32_t>, cugraph::edge_dummy_property_view_t, e_op_t<thrust::tuple<int32_t, int32_t>, int32_t, int32_t, int32_t>)
          cugraph::extract_transform_e(*handle_,
          ^

The error from clang tells us the other half:

extract_transform_e.cuh(91, 1): Candidate template ignored: substitution failure [with
  GraphViewType = graph_view_t<int, int, false, true>,
  EdgeSrcValueInputWrapper = edge_major_property_view_t<int, const_value_iterator, int>,
  EdgeDstValueInputWrapper = edge_minor_property_view_t<int, const_iterator, int>,
  EdgeValueInputWrapper = edge_dummy_property_view_t,
  EdgeOp = e_op_t<key_t, int, result_t, int>
]: implicit instantiation of undefined template 'cugraph::detail::edge_op_result_type<int, int, int, int, thrust::nullopt_t, e_op_t<int, int, int, int>>'

Combining info from both errors, the issue is that when extract_transform_e instantiates detail::edge_op_result_type, it tries to get the return type of invoking e_opt_t with int, int, int, int.

But e_opt_t was declared with template parameters thrust::tuple<int32_t, int32_t>, int32_t, int32_t, int32_t, and since thrust tuples can no longer be default constructed from a scalar, the operator()() overload cannot be invoked with int as the first argument.

trxcllnt avatar May 09 '24 22:05 trxcllnt

The new kv_store_t constructor is necessary because there are code paths that attempt to construct a thrust::tuple from a scalar (invalid_vertex_id<vertex_t>::value here).

While this is possible in CCCL 2.2.0's version of thrust::tuple, I checked with @jrhemstad it was definitely never intended to work. CCCL 2.4+ makes thrust::tuple an alias of cuda::std::tuple, so this behavior is now gone, and cuGraph needs to be updated.

I commented out the prims/mg_extract_transform_e.cu test for now, because the compiler resisted my attempts to rewrite this functor to be compatible with the new CCCL behavior.

The problem case is when the key_t template parameter is thrust::tuple<vertex_t, int32_t>, but the functor is called with the invalid value scalar -1.

Alternatively, I attempted to specialize the invalid_idx struct to return a thrust::tuple if the vertex_t template parameter is a tuple, but that seemed to break in other ways.

cc: @BradReesWork if you have any ideas or know the right person to help fix this.

@seunghwak is our expert in this area

ChuckHastings avatar May 10 '24 15:05 ChuckHastings

It looks like the main build is working, but the wheel builds are blocked by https://github.com/rapidsai/cugraph/pull/4436?

trxcllnt avatar May 24 '24 03:05 trxcllnt

/merge

trxcllnt avatar May 29 '24 18:05 trxcllnt