remove pushpull option from BFS (always do push/pull)

doc/TODO.txt

@@ -14,8 +14,6 @@ As of Feb 14, 2022:
 
     * TODOs: in algorithms
 
-        * LAGraph_BreadthFirstSearch: pushpull option
-
         * LAGraph_VertexCentrality: wrapper for 2 methods (BC, PageRank),
             and future methods.
             How do we ask for an exact BC computation?  Do it in batches

include/LAGraph.h

@@ -1619,6 +1619,8 @@ int LAGraph_Sort3
 // LAGraph_BreadthFirstSearch: breadth-first search
 //------------------------------------------------------------------------------
 
+// This is an Advanced algorithm (G->AT and G->rowdegree required).
+
 /*
  * Perform breadth-first traversal, computing parent vertex ID's
  * and/or level encountered.
@@ -1635,24 +1637,12 @@ int LAGraph_Sort3
  *                           The parent vector is not computed if NULL.
  * @param[in]     G          The graph, directed or undirected.
  * @param[in]     src        The index of the src vertex (0-based)
- * @param[in]     pushpull   if true, use push/pull; otherwise, use pushonly.
- *                           Push/pull is faster but requires G->AT,
- *                           G->rowdegree, and SuiteSparse:GraphBLAS.
- *                TODO: consider removing pushpull option or reverse logic
  * @param[out]    msg        Error message if a failure code is returned.
  *
  * @retval GrB_SUCCESS      successful
  * @retval LAGRAPH_INVALID_GRAPH Graph is invalid (LAGraph_CheckGraph failed)
  */
 
-// TODO: direction-optimization (pushpull) requires G->AT and G->rowdgree.
-// If not present, a Basic method could compute them and then use pushpull.  Or
-// it could just use push-only.  The current method, below, doesn't modify G so
-// it's a Basic method, but it requires the pushpull input parameter.  Perhaps
-// a Basic method should just use pushpull as true.  An Advanced method would
-// not need G->AT or G->rowdegree if pushpull is false; if pushpull is true, it
-// could return an error if G->AT or G->rowdegree are not present.
-
 LAGRAPH_PUBLIC
 int LAGraph_BreadthFirstSearch
 (
@@ -1662,7 +1652,6 @@ int LAGraph_BreadthFirstSearch
     // input:
     LAGraph_Graph  G,
     GrB_Index      src,
-    bool           pushpull,
     char          *msg
 ) ;
 

src/algorithm/LAGraph_BreadthFirstSearch.c

@@ -14,10 +14,6 @@
 // and its GxB extensions, or a push-only method otherwise.  The former is
 // much faster.
 
-// TODO: consider removing the pushpull option or reverse logic.
-// If we remove it, the option should be true here.  It could still be
-// included in the LG* methods.
-
 #include "LG_alg_internal.h"
 
 int LAGraph_BreadthFirstSearch
@@ -28,15 +24,14 @@ int LAGraph_BreadthFirstSearch
     // input:
     LAGraph_Graph  G,
     GrB_Index      src,
-    bool           pushpull,
     char          *msg
 )
 {
 
 #if LG_SUITESPARSE
-    return LG_BreadthFirstSearch_SSGrB  (level, parent, G, src, pushpull, msg);
+    return LG_BreadthFirstSearch_SSGrB   (level, parent, G, src, msg) ;
 #else
-    return LG_BreadthFirstSearch_vanilla(level, parent, G, src, msg);
+    return LG_BreadthFirstSearch_vanilla (level, parent, G, src, msg) ;
 #endif
 }
 

src/algorithm/LAGraph_VertexCentrality.c

@@ -5,6 +5,8 @@
 // LAGraph, (c) 2021 by The LAGraph Contributors, All Rights Reserved.
 // SPDX-License-Identifier: BSD-2-Clause
 
+// Contributed by Tim Davis, Texas A&M University
+
 //------------------------------------------------------------------------------
 
 // LAGraph_VertexCentrality is a Basic method for computing many types of
@@ -24,15 +26,13 @@
 // and thus the methods can be used to compute approximate metrics instead,
 // which is much faster.
 
-// TODO: only methods 1 and 2 are currently implemented.  Method (2) currently
-// uses the GAP algorithm, which ignores dangling nodes (we need a normal
-// pagerank).  No method considers edge weights.
+// TODO: only methods 1 and 2 are currently implemented.
+// Neither method considers edge weights, but edge-weights could be handled.
 
 // TODO: do we ensure that sum(centrality) == 1 for all methods?
 
 #include "LG_alg_internal.h"
 
-// TODO the following is a draft:
 int LAGraph_VertexCentrality    // TODO: write this
 (
     // output:
@@ -48,3 +48,4 @@ int LAGraph_VertexCentrality    // TODO: write this
     LG_CLEAR_MSG ;
     return (GrB_NOT_IMPLEMENTED) ;        // TODO
 }
+

src/algorithm/LG_BreadthFirstSearch_SSGrB.c

@@ -44,7 +44,6 @@ int LG_BreadthFirstSearch_SSGrB
     GrB_Vector    *parent,
     LAGraph_Graph  G,
     GrB_Index      src,
-    bool           pushpull,
     char          *msg
 )
 {
@@ -103,10 +102,15 @@ int LG_BreadthFirstSearch_SSGrB
     {
         // AT = A' is different from A
         AT = G->AT ;
+        LG_ASSERT_MSG (AT != NULL,
+            LAGRAPH_PROPERTY_MISSING, "G->AT is required") ;
     }
 
-    // direction-optimization requires AT and Degree
-    bool push_pull = pushpull && (AT != NULL) && (Degree != NULL) ;
+    // direction-optimization requires G->AT and G->rowdegree
+    LG_ASSERT_MSG (Degree != NULL,
+        LAGRAPH_PROPERTY_MISSING, "G->rowdegree is required") ;
+
+    bool push_pull = true ;
 
     // determine the semiring type
     GrB_Type int_type = (n > INT32_MAX) ? GrB_INT64 : GrB_INT32 ;
@@ -148,11 +152,8 @@ int LG_BreadthFirstSearch_SSGrB
         GrB_TRY (GrB_Vector_setElement (v, 0, src)) ;
     }
 
-    if (push_pull)
-    {
-        // workspace for computing work remaining
-        GrB_TRY (GrB_Vector_new (&w, GrB_INT64, n)) ;
-    }
+    // workspace for computing work remaining
+    GrB_TRY (GrB_Vector_new (&w, GrB_INT64, n)) ;
 
     GrB_Index nq = 1 ;          // number of nodes in the current level
     double alpha = 8.0 ;

src/algorithm/LG_alg_internal.h

@@ -23,7 +23,6 @@ int LG_BreadthFirstSearch_SSGrB
     // input:
     LAGraph_Graph  G,
     GrB_Index      src,
-    bool           pushpull,
     char          *msg
 ) ;
 

src/benchmark/README.md

@@ -134,7 +134,7 @@ files (SuiteSparse-only), and to run the GAP benchmarks, do the following:
     you will see 6 runs per matrix on stderr.  The default "black box" GAP
     benchmark results are the "parent only pushpull" results; the other results
     are for different problems (such as level-only, or parent+level), or with a
-    different algorithm that is typically non-optimal (pushonly).  However, the
+    different algorithm that is typically non-optimal (pushonly).  However, a
     pushonly BFS for the GAP-road graph is typically faster than the pushpull
     method, since the heuristic for push vs pull always selects the pull phase.
 

src/benchmark/bfs_demo.c

@@ -72,9 +72,9 @@ int main (int argc, char **argv)
     }
     printf ("\n") ;
 
-    double tpl [nthreads_max+1][2] ;
-    double tp [nthreads_max+1][2] ;
-    double tl [nthreads_max+1][2] ;
+    double tpl [nthreads_max+1] ;
+    double tp [nthreads_max+1] ;
+    double tl [nthreads_max+1] ;
 
     //--------------------------------------------------------------------------
     // read in the graph
@@ -111,8 +111,7 @@ int main (int argc, char **argv)
     double twarmup, tw [2] ;
     GrB_TRY (GrB_Matrix_extractElement (&src, SourceNodes, 0, 0)) ;
     LAGraph_TRY (LAGraph_Tic (tw, msg)) ;
-    LAGraph_TRY (LAGraph_BreadthFirstSearch (NULL, &parent,
-        G, src, false, msg)) ;
+    LAGraph_TRY (LAGraph_BreadthFirstSearch (NULL, &parent, G, src, msg)) ;
     GrB_free (&parent) ;
     LAGraph_TRY (LAGraph_Toc (&twarmup, tw, msg)) ;
     printf ("warmup: parent only, pushonly: %g sec\n", twarmup) ;
@@ -127,13 +126,9 @@ int main (int argc, char **argv)
         if (nthreads > nthreads_max) continue ;
         LAGraph_TRY (LAGraph_SetNumThreads (nthreads, msg)) ;
 
-        tp [nthreads][0] = 0 ;
-        tl [nthreads][0] = 0 ;
-        tpl [nthreads][0] = 0 ;
-
-        tp [nthreads][1] = 0 ;
-        tl [nthreads][1] = 0 ;
-        tpl [nthreads][1] = 0 ;
+        tp [nthreads] = 0 ;
+        tl [nthreads] = 0 ;
+        tpl [nthreads] = 0 ;
 
         printf ("\n------------------------------- threads: %2d\n", nthreads) ;
         for (int trial = 0 ; trial < ntrials ; trial++)
@@ -144,25 +139,20 @@ int main (int argc, char **argv)
             src-- ; // convert from 1-based to 0-based
             double tcheck, ttrial, tic [2] ;
 
-            // for (int pp = 0 ; pp <= 1 ; pp++)
-            for (int pp = 1 ; pp <= 1 ; pp++)
             {
 
-                bool pushpull = (pp == 1) ;
-
                 //--------------------------------------------------------------
                 // BFS to compute just parent
                 //--------------------------------------------------------------
 
                 GrB_free (&parent) ;
                 LAGraph_TRY (LAGraph_Tic (tic, msg)) ;
                 LAGraph_TRY (LAGraph_BreadthFirstSearch (NULL, &parent,
-                    G, src, pushpull, msg)) ;
+                    G, src, msg)) ;
                 LAGraph_TRY (LAGraph_Toc (&ttrial, tic, msg)) ;
-                tp [nthreads][pp] += ttrial ;
-                printf ("parent only  %s trial: %2d threads: %2d "
+                tp [nthreads] += ttrial ;
+                printf ("parent only  pushpull trial: %2d threads: %2d "
                     "src: %g %10.4f sec\n",
-                    (pp == 0) ? "pushonly" : "pushpull",
                     trial, nthreads, (double) src, ttrial) ;
                 fflush (stdout) ;
 
@@ -191,15 +181,14 @@ int main (int argc, char **argv)
 
                 LAGraph_TRY (LAGraph_Tic (tic, msg)) ;
                 LAGraph_TRY (LAGraph_BreadthFirstSearch (&level, NULL,
-                    G, src, pushpull, msg)) ;
+                    G, src, msg)) ;
                 LAGraph_TRY (LAGraph_Toc (&ttrial, tic, msg)) ;
-                tl [nthreads][pp] += ttrial ;
+                tl [nthreads] += ttrial ;
 
                 GrB_TRY (GrB_reduce (&maxlevel, NULL, GrB_MAX_MONOID_INT32,
                     level, NULL)) ;
-                printf ("level only   %s trial: %2d threads: %2d "
+                printf ("level only   pushpull trial: %2d threads: %2d "
                     "src: %g %10.4f sec maxlevel: %d\n",
-                    (pp == 0) ? "pushonly" : "pushpull",
                     trial, nthreads, (double) src, ttrial, maxlevel) ;
                 fflush (stdout) ;
 
@@ -227,15 +216,14 @@ int main (int argc, char **argv)
                 GrB_free (&level) ;
                 LAGraph_TRY (LAGraph_Tic (tic, msg)) ;
                 LAGraph_TRY (LAGraph_BreadthFirstSearch (&level, &parent,
-                    G, src, pushpull, msg)) ;
+                    G, src, msg)) ;
                 LAGraph_TRY (LAGraph_Toc (&ttrial, tic, msg)) ;
-                tpl [nthreads][pp] += ttrial ;
+                tpl [nthreads] += ttrial ;
 
                 GrB_TRY (GrB_reduce (&maxlevel, NULL, GrB_MAX_MONOID_INT32,
                     level, NULL)) ;
-                printf ("parent+level %s trial: %2d threads: %2d "
+                printf ("parent+level pushpull trial: %2d threads: %2d "
                     "src: %g %10.4f sec\n",
-                    (pp == 0) ? "pushonly" : "pushpull",
                     trial, nthreads, (double) src, ttrial) ;
                 fflush (stdout) ;
 
@@ -259,44 +247,36 @@ int main (int argc, char **argv)
             }
         }
 
-        // for (int pp = 0 ; pp <= 1 ; pp++)
-        for (int pp = 1 ; pp <= 1 ; pp++)
         {
-            tp  [nthreads][pp] = tp  [nthreads][pp] / ntrials ;
-            tl  [nthreads][pp] = tl  [nthreads][pp] / ntrials ;
-            tpl [nthreads][pp] = tpl [nthreads][pp] / ntrials ;
+            tp  [nthreads] = tp  [nthreads] / ntrials ;
+            tl  [nthreads] = tl  [nthreads] / ntrials ;
+            tpl [nthreads] = tpl [nthreads] / ntrials ;
 
-            fprintf (stderr, "Avg: BFS %s parent only  threads %3d: "
+            fprintf (stderr, "Avg: BFS pushpull parent only  threads %3d: "
                 "%10.3f sec: %s\n",
-                 (pp == 0) ? "pushonly" : "pushpull",
-                 nthreads, tp [nthreads][pp], matrix_name) ;
+                 nthreads, tp [nthreads], matrix_name) ;
 #if 0
-            fprintf (stderr, "Avg: BFS %s level only   threads %3d: "
+            fprintf (stderr, "Avg: BFS pushpull level only   threads %3d: "
                 "%10.3f sec: %s\n",
-                 (pp == 0) ? "pushonly" : "pushpull",
-                 nthreads, tl [nthreads][pp], matrix_name) ;
+                 nthreads, tl [nthreads], matrix_name) ;
 
-            fprintf (stderr, "Avg: BFS %s level+parent threads %3d: "
+            fprintf (stderr, "Avg: BFS pushpull level+parent threads %3d: "
                 "%10.3f sec: %s\n",
-                 (pp == 0) ? "pushonly" : "pushpull",
-                 nthreads, tpl [nthreads][pp], matrix_name) ;
+                 nthreads, tpl [nthreads], matrix_name) ;
 #endif
 
-            printf ("Avg: BFS %s parent only  threads %3d: "
+            printf ("Avg: BFS pushpull parent only  threads %3d: "
                 "%10.3f sec: %s\n",
-                 (pp == 0) ? "pushonly" : "pushpull",
-                 nthreads, tp [nthreads][pp], matrix_name) ;
+                 nthreads, tp [nthreads], matrix_name) ;
 
 #if 0
-            printf ("Avg: BFS %s level only   threads %3d: "
+            printf ("Avg: BFS pushpull level only   threads %3d: "
                 "%10.3f sec: %s\n",
-                 (pp == 0) ? "pushonly" : "pushpull",
-                 nthreads, tl [nthreads][pp], matrix_name) ;
+                 nthreads, tl [nthreads], matrix_name) ;
 
-            printf ("Avg: BFS %s level+parent threads %3d: "
+            printf ("Avg: BFS pushpull level+parent threads %3d: "
                 "%10.3f sec: %s\n",
-                 (pp == 0) ? "pushonly" : "pushpull",
-                 nthreads, tpl [nthreads][pp], matrix_name) ;
+                 nthreads, tpl [nthreads], matrix_name) ;
 #endif
         }
     }