Merge pull request #4125 from bhandras/betweenness_centrality

Addressing final comments from beteweenness centrality PR

autopilot/betweenness_centrality.go

@@ -98,13 +98,13 @@ func betweennessCentrality(g *SimpleGraph, s int, centrality []float64) {
 	// shortest path from s to t for each node t.
 	pred := make([][]int, len(g.Nodes))
 
-	// sigma[t] is the number of shortest paths between nodes s and t for
+	// sigma[t] is the number of shortest paths between nodes s and t
-	// each node t.
+	// for each node t.
 	sigma := make([]int, len(g.Nodes))
 	sigma[s] = 1
 
-	// dist[t] holds the distance between s and t for each node t. We initialize
+	// dist[t] holds the distance between s and t for each node t.
-	// this to -1 (meaning infinity) for each t != s.
+	// We initialize this to -1 (meaning infinity) for each t != s.
 	dist := make([]int, len(g.Nodes))
 	for i := range dist {
 		dist[i] = -1
@@ -178,9 +178,9 @@ func (bc *BetweennessCentrality) Refresh(graph ChannelGraph) error {
 	work := make(chan int)
 	partials := make(chan []float64, bc.workers)
 
-	// Each worker will compute a partial result. This
+	// Each worker will compute a partial result.
-	// partial result is a sum of centrality updates on
+	// This partial result is a sum of centrality updates
-	// roughly N / workers nodes.
+	// on roughly N / workers nodes.
 	worker := func() {
 		defer wg.Done()
 		partial := make([]float64, len(cache.Nodes))
@@ -199,8 +199,8 @@ func (bc *BetweennessCentrality) Refresh(graph ChannelGraph) error {
 		go worker()
 	}
 
-	// Distribute work amongst workers Should be
+	// Distribute work amongst workers.
-	// fair when graph is sufficiently large.
+	// Should be fair when the graph is sufficiently large.
 	for node := range cache.Nodes {
 		work <- node
 	}

autopilot/betweenness_centrality_test.go

@@ -91,7 +91,7 @@ func buildTestGraph(t *testing.T,
 		for _, v := range neighbors {
 			_, _, err := graph.addRandChannel(nodes[u], nodes[v], chanCapacity)
 			if err != nil {
-				t.Fatalf("unexpected error while adding random channel: %v", err)
+				t.Fatalf("unexpected error adding random channel: %v", err)
 			}
 		}
 	}
@@ -147,9 +147,12 @@ func TestBetweennessCentralityWithNonEmptyGraph(t *testing.T) {
 
 			testName := fmt.Sprintf("%v %d workers", chanGraph.name, numWorkers)
 			success := t.Run(testName, func(t1 *testing.T) {
-				centralityMetric, err := NewBetweennessCentralityMetric(numWorkers)
+				centralityMetric, err := NewBetweennessCentralityMetric(
+					numWorkers,
+				)
 				if err != nil {
-					t.Fatalf("construction must succeed with positive number of workers")
+					t.Fatalf("construction must succeed with " +
+						"positive number of workers")
 				}
 
 				graphNodes := buildTestGraph(t1, graph, graphDesc)
@@ -169,11 +172,13 @@ func TestBetweennessCentralityWithNonEmptyGraph(t *testing.T) {
 						nodeID := NewNodeID(graphNodes[node])
 						calculatedCentrality, ok := centrality[nodeID]
 						if !ok {
-							t1.Fatalf("no result for node: %x (%v)", nodeID, node)
+							t1.Fatalf("no result for node: %x (%v)",
+								nodeID, node)
 						}
 
 						if nodeCentrality != calculatedCentrality {
-							t1.Errorf("centrality for node: %v should be %v, got: %v",
+							t1.Errorf("centrality for node: %v "+
+								"should be %v, got: %v",
 								node, nodeCentrality, calculatedCentrality)
 						}
 					}

lnrpc/rpc.proto

@@ -651,7 +651,6 @@ service Lightning {
         };
     }
 
-
     /** lncli: `getchaninfo`
     GetChanInfo returns the latest authenticated network announcement for the
     given channel identified by its channel ID: an 8-byte integer which
@@ -2527,13 +2526,14 @@ message ChannelGraph {
     repeated ChannelEdge edges = 2;
 }
 
-enum NodeMetricType {
+enum NodeMetricType { 
-    BETWEENNESS_CENTRALITY = 0;
+    UNKNOWN = 0;
+    BETWEENNESS_CENTRALITY = 1; 
 }
 
 message NodeMetricsRequest {
-  /// The requesteded node metrics.
+    /// The requested node metrics.
-  repeated NodeMetricType types = 1;
+    repeated NodeMetricType types = 1;
 }
 
 message NodeMetricsResponse {
@@ -2544,15 +2544,15 @@ message NodeMetricsResponse {
     Map of node pubkey to betweenness centrality of the node. Normalized
     values are in the [0,1] closed interval.
     */
-    map<string, FloatValue> betweenness_centrality = 1;
+    map<string, FloatMetric> betweenness_centrality = 1;
 }
 
-message FloatValue {
+message FloatMetric {
-  /// Arbitrary float value.
+    /// Arbitrary float value.
-  double value = 1;
+    double value = 1;
 
-  /// The value normalized to [0,1] or [-1,1].
+    /// The value normalized to [0,1] or [-1,1].
-  double normalized_value = 2;
+    double normalized_value = 2;
 }
 
 message ChanInfoRequest {

lnrpc/rpc.swagger.json

@@ -733,13 +733,14 @@
         "parameters": [
           {
             "name": "types",
-            "description": "/ The requesteded node metrics.",
+            "description": "/ The requested node metrics.",
             "in": "query",
             "required": false,
             "type": "array",
             "items": {
               "type": "string",
               "enum": [
+                "UNKNOWN",
                 "BETWEENNESS_CENTRALITY"
               ]
             },
@@ -2717,7 +2718,7 @@
         }
       }
     },
-    "lnrpcFloatValue": {
+    "lnrpcFloatMetric": {
       "type": "object",
       "properties": {
         "value": {
@@ -3625,9 +3626,10 @@
     "lnrpcNodeMetricType": {
       "type": "string",
       "enum": [
+        "UNKNOWN",
         "BETWEENNESS_CENTRALITY"
       ],
-      "default": "BETWEENNESS_CENTRALITY"
+      "default": "UNKNOWN"
     },
     "lnrpcNodeMetricsResponse": {
       "type": "object",
@@ -3635,7 +3637,7 @@
         "betweenness_centrality": {
           "type": "object",
           "additionalProperties": {
-            "$ref": "#/definitions/lnrpcFloatValue"
+            "$ref": "#/definitions/lnrpcFloatMetric"
           },
           "description": "*\nBetweenness centrality is the sum of the ratio of shortest paths that pass\nthrough the node for each pair of nodes in the graph (not counting paths\nstarting or ending at this node).\nMap of node pubkey to betweenness centrality of the node. Normalized\nvalues are in the [0,1] closed interval."
         }

rpcserver.go

@@ -4688,7 +4688,7 @@ func (r *rpcServer) GetNodeMetrics(ctx context.Context,
 	}
 
 	resp := &lnrpc.NodeMetricsResponse{
-		BetweennessCentrality: make(map[string]*lnrpc.FloatValue),
+		BetweennessCentrality: make(map[string]*lnrpc.FloatMetric),
 	}
 
 	// Obtain the pointer to the global singleton channel graph, this will
@@ -4712,9 +4712,10 @@ func (r *rpcServer) GetNodeMetrics(ctx context.Context,
 	// Fill normalized and non normalized centrality.
 	centrality := centralityMetric.GetMetric(true)
 	for nodeID, val := range centrality {
-		resp.BetweennessCentrality[hex.EncodeToString(nodeID[:])] = &lnrpc.FloatValue{
+		resp.BetweennessCentrality[hex.EncodeToString(nodeID[:])] =
-			NormalizedValue: val,
+			&lnrpc.FloatMetric{
-		}
+				NormalizedValue: val,
+			}
 	}
 
 	centrality = centralityMetric.GetMetric(false)