Address PR comments, restructure C API implementation more cleanly

Author: ChuckHastings

cpp/CMakeLists.txt

@@ -679,6 +679,7 @@ add_library(cugraph_c
         src/c_api/lookup_src_dst.cpp
         src/c_api/louvain.cpp
         src/c_api/triangle_count.cpp
+        src/c_api/neighbor_sample.cpp
         src/c_api/neighbor_sampling.cpp
         src/c_api/sampling_result.cpp
         src/c_api/temporal_neighbor_sampling.cpp

cpp/include/cugraph/sampling_functions.hpp

@@ -44,19 +44,26 @@ enum class temporal_sampling_comparison_t {
   MONOTONICALLY_DECREASING, /** Time monotonically decreasing (could have multiple edges with same
                                 time) */
   FIXED_WINDOW,             /** Apply the original per-seed time window at every hop */
-  LAST = FIXED_WINDOW       /** Deprecated alias for FIXED_WINDOW */
+  LAST                      /** Deprecated Value, we moved last-n to a different parameter */
 };
 
 /**
  * @brief Selects neighbors from the set that satisfies the sampling filters.
  */
 enum class neighbor_selection_t {
   RANDOM = 0, /** Random selection. Uniform if no bias view is supplied, biased otherwise. */
-  FIRST,      /** Deterministically select the earliest edges. Not yet implemented. */
-  LAST        /** Deterministically select the latest edges. Not yet implemented. */
+  LAST /** Deterministically select the latest edges based on the ordering criteria defined in
+          temporal_sampling_comparison. Not yet implemented. */
 };
 
-struct sampling_flags_t {
+/**
+ * @brief Options controlling neighborhood sampling behavior.
+ *
+ * Temporal sampling is enabled when @p temporal_sampling_comparison has a value; leave it
+ * unset for non-temporal sampling. Future sampling knobs should be added here so
+ * `neighbor_sample` can keep a stable parameter list.
+ */
+struct sampling_options_t {
   /**
    * Specifies how to handle prior sources. Default is DEFAULT.
    */
@@ -81,10 +88,10 @@ struct sampling_flags_t {
   bool with_replacement{true};
 
   /**
-   * Specifies how to handle temporal sampling. Default is STRICTLY_INCREASING.
+   * When set, enables temporal sampling with the given comparison mode. Default is unset
+   * (non-temporal).
    */
-  temporal_sampling_comparison_t temporal_sampling_comparison{
-    temporal_sampling_comparison_t::STRICTLY_INCREASING};
+  std::optional<temporal_sampling_comparison_t> temporal_sampling_comparison{std::nullopt};
 
   /**
    * Specifies if disjoint sampling should be enforced. Default is false.
@@ -97,6 +104,9 @@ struct sampling_flags_t {
   neighbor_selection_t neighbor_selection{neighbor_selection_t::RANDOM};
 };
 
+/** @deprecated Use sampling_options_t. */
+using sampling_flags_t = sampling_options_t;
+
 /**
  * @ingroup sampling_functions_cpp
  * @deprecated Use neighbor_sample instead.
@@ -925,11 +935,10 @@ heterogeneous_biased_temporal_neighbor_sample(
  * contains one value per hop.
  *
  * RANDOM selection samples uniformly when @p edge_bias_view is absent and samples according to
- * the supplied biases when it is present. FIRST and LAST are reserved for deterministically
- * selecting the earliest or latest eligible edges (temporal only; no bias; no with-replacement),
- * but are not yet implemented.
+ * the supplied biases when it is present. LAST is reserved for deterministically selecting the
+ * latest eligible edges (temporal only; no bias; no with-replacement), but is not yet implemented.
  *
- * Sampling is temporal when @p temporal_sampling_comparison is specified; in that case
+ * Sampling is temporal when @p sampling_options.temporal_sampling_comparison is set; in that case
  * @p edge_start_time_view is required. FIXED_WINDOW applies each seed's original closed time window
  * at every hop, while the increasing and decreasing modes propagate sampled edge times as the next
  * frontier bound.
@@ -967,9 +976,7 @@ neighbor_sample(
   std::optional<raft::device_span<int32_t const>> label_to_output_comm_rank,
   raft::host_span<int32_t const> fan_out,
   std::optional<edge_type_t> num_edge_types,
-  neighbor_selection_t neighbor_selection,
-  std::optional<temporal_sampling_comparison_t> temporal_sampling_comparison,
-  sampling_flags_t sampling_flags,
+  sampling_options_t sampling_options,
   bool do_expensive_check = false);
 
 /**

cpp/include/cugraph_c/sampling_algorithms.h

@@ -198,18 +198,17 @@ typedef enum {
   MONOTONICALLY_DECREASING, /** Time monotonically decreasing (could have multiple edges with same
                                 time) */
   FIXED_WINDOW,             /** Apply the original per-seed time window at every hop */
-  LAST = FIXED_WINDOW       /** Deprecated alias for FIXED_WINDOW */
+  LAST                      /** Deprecated Value, we moved last-n to a different parameter */
 } cugraph_temporal_sampling_comparison_t;
 
 /**
  * @brief Selects neighbors from the set that satisfies the sampling filters.
  */
 typedef enum {
   CUGRAPH_NEIGHBOR_SELECTION_RANDOM = 0, /** Uniform without biases, biased with biases */
-  CUGRAPH_NEIGHBOR_SELECTION_FIRST,      /** Deterministically select the earliest edges (not yet
-                                            implemented) */
-  CUGRAPH_NEIGHBOR_SELECTION_LAST        /** Deterministically select the latest edges (not yet
-                                            implemented) */
+  CUGRAPH_NEIGHBOR_SELECTION_LAST        /** Deterministically select the last-n edges based on the
+                                            ordering criteria defined in temporal_sampling_comparison (not
+                                            yet        implemented) */
 } cugraph_neighbor_selection_t;
 
 /**
@@ -330,6 +329,25 @@ CUGRAPH_EXPORT void cugraph_sampling_set_dedupe_sources(cugraph_sampling_options
 CUGRAPH_EXPORT void cugraph_sampling_set_temporal_sampling_comparison(
   cugraph_sampling_options_t* options, cugraph_temporal_sampling_comparison_t comparison);
 
+/**
+ * @ingroup samplingC
+ * @brief Clear temporal sampling comparison, disabling temporal sampling.
+ *
+ * @param options - opaque pointer to the sampling options
+ */
+CUGRAPH_EXPORT void cugraph_sampling_clear_temporal_sampling_comparison(
+  cugraph_sampling_options_t* options);
+
+/**
+ * @ingroup samplingC
+ * @brief Set neighbor selection mode.
+ *
+ * @param options - opaque pointer to the sampling options
+ * @param selection - RANDOM or LAST
+ */
+CUGRAPH_EXPORT void cugraph_sampling_set_neighbor_selection(cugraph_sampling_options_t* options,
+                                                            cugraph_neighbor_selection_t selection);
+
 /**
  * @ingroup samplingC
  * @brief   Set flag to perform disjoint sampling
@@ -354,14 +372,14 @@ CUGRAPH_EXPORT void cugraph_sampling_options_free(cugraph_sampling_options_t* op
  * @brief Unified homogeneous/heterogeneous and temporal/non-temporal neighborhood sampling.
  *
  * RANDOM selection samples uniformly if @p edge_biases is NULL and samples according to
- * @p edge_biases otherwise. FIRST and LAST are reserved for deterministically selecting the
- * earliest or latest temporally eligible edges (requiring @p temporal_sampling_comparison), but
- * are not yet implemented.
+ * @p edge_biases otherwise. LAST is reserved for deterministically selecting the latest
+ * temporally eligible edges (requiring temporal sampling), but is not yet implemented.
  *
- * Sampling is non-temporal when @p temporal_sampling_comparison is NULL; all temporal arguments
- * must then be NULL. Sampling is homogeneous when @p num_edge_types is 1; otherwise
- * @p fan_out contains one value per (hop, edge type). When @p vertex_type_offsets is NULL,
- * all vertices are treated as a single vertex type.
+ * Sampling is non-temporal unless cugraph_sampling_set_temporal_sampling_comparison has been
+ * called on @p sampling_options; all temporal arguments must then be NULL. Sampling is
+ * homogeneous when @p num_edge_types is 1; otherwise @p fan_out contains one value per
+ * (hop, edge type). When @p vertex_type_offsets is NULL, all vertices are treated as a single
+ * vertex type.
  *
  * @param [in] handle Handle for accessing resources.
  * @param [in,out] rng_state State of the random number generator, updated for RANDOM selection.
@@ -375,10 +393,9 @@ CUGRAPH_EXPORT void cugraph_sampling_options_free(cugraph_sampling_options_t* op
  * processing.
  * @param [in] fan_out Host array defining fanout per hop (and per edge type when heterogeneous).
  * @param [in] num_edge_types Number of edge types. Use 1 for homogeneous sampling.
- * @param [in] neighbor_selection RANDOM, FIRST, or LAST.
- * @param [in] temporal_sampling_comparison Optional temporal comparison. NULL disables temporal
- * sampling.
- * @param [in] sampling_options Sampling options.
+ * @param [in] sampling_options Sampling options. Temporal sampling is enabled by
+ * cugraph_sampling_set_temporal_sampling_comparison; neighbor selection defaults to RANDOM and may
+ * be set with cugraph_sampling_set_neighbor_selection.
  * @param [in] do_expensive_check Whether to perform expensive input validation.
  * @param [out] result Sampling result.
  * @param [out] error Error details on failure.
@@ -396,8 +413,6 @@ CUGRAPH_EXPORT cugraph_error_code_t cugraph_neighbor_sample(
   const cugraph_type_erased_device_array_view_t* vertex_type_offsets,
   const cugraph_type_erased_host_array_view_t* fan_out,
   int num_edge_types,
-  cugraph_neighbor_selection_t neighbor_selection,
-  const cugraph_temporal_sampling_comparison_t* temporal_sampling_comparison,
   const cugraph_sampling_options_t* sampling_options,
   bool_t do_expensive_check,
   cugraph_sample_result_t** result,