Merge pull request #558 from fabratu/fix-ctd

Fix: Commute Time Distance

include/networkit/distance/CommuteTimeDistance.hpp

@@ -56,11 +56,6 @@ class CommuteTimeDistance final : public Algorithm {
      */
     uint64_t getSetupTime() const;
 
-    /**
-     * Returns the commute time distance between node @a u and node @a v.
-     * @return commute time distance between the two nodes. Needs to call run() or runApproximation() first.
-     */
-    double distance(node u, node v);
 
     /**
      * Returns the commute time distance between node @a u and node @a v.
@@ -71,10 +66,11 @@ class CommuteTimeDistance final : public Algorithm {
     double runSinglePair(node u, node v);
 
     /**
-     * Returns the sum of the distances from node @a u.
-     * This method does not need the initial preprocessing.
-     * @return commute sum of the distances from the node.
+     * Returns the commute time distance between node @a u and node @a v.
+     * @return commute time distance between the two nodes. Needs to call run() or runApproximation() first.
      */
+    double distance(node u, node v);
+
     double runSingleSource(node u);
 
 protected:

include/networkit/graph/GraphTools.hpp

@@ -128,6 +128,14 @@ std::pair<node, node> size(const Graph &G) noexcept;
  */
 double density(const Graph &G) noexcept;
 
+/**
+ * Returns the volume (sum of the out-degree of all nodes) of the graph.
+ *
+ * @param G The input graph.
+ * @return Volume of the graph.
+ */
+double volume(const Graph &G);
+
 /**
  * Copies all nodes of the input graph to a new graph (edges are not copied).
  *

networkit/_NetworKit.pyx

@@ -5236,6 +5236,7 @@ cdef extern from "<networkit/graph/GraphTools.hpp>" namespace "NetworKit::GraphT
 	vector[pair[node, node]] randomEdges(_Graph G, count numEdges) nogil except +
 	pair[count, count] size(_Graph G) nogil except +
 	double density(_Graph G) nogil except +
+	double volume(_Graph G) nogil except +
 	_Graph copyNodes(_Graph G) nogil except +
 	_Graph toUndirected(_Graph G) nogil except +
 	_Graph toUnweighted(_Graph G) nogil except +
@@ -5529,6 +5530,23 @@ cdef class GraphTools:
 		"""
 		return density(graph._this)
 
+	@staticmethod
+	def volume(Graph graph):
+		"""
+		Get the volume of the input graph.
+
+		Parameters
+		----------
+		graph : networkit.Graph
+			The input graph.
+
+		Returns
+		-------
+		double
+			The volume of the input graph.
+		"""
+		return volume(graph._this)
+
 	@staticmethod
 	def copyNodes(Graph graph):
 		"""

networkit/cpp/distance/CommuteTimeDistance.cpp

@@ -11,12 +11,15 @@
 #include <networkit/auxiliary/Log.hpp>
 #include <networkit/auxiliary/Timer.hpp>
 #include <networkit/distance/CommuteTimeDistance.hpp>
+#include <networkit/graph/GraphTools.hpp>
 
 namespace NetworKit {
 
 CommuteTimeDistance::CommuteTimeDistance(const Graph& G, double tol): Algorithm(), G(&G), tol(tol), lamg(1e-5) {
     // main purpose of method: preparing LAMG
-
+    if(G.isDirected()) 
+        throw std::runtime_error("Commute time distance is only supported for undirected graphs.");
+
     // construct matrix from graph
     CSRMatrix matrix = CSRMatrix::laplacianMatrix(G);
 
@@ -150,14 +153,7 @@ void CommuteTimeDistance::runParallelApproximation() {
 double CommuteTimeDistance::distance(node u, node v) {
     assureFinished();
 
-    // compute volume
-    double volG = 0.0;
-    if (! G->isWeighted()) {
-        volG = 2.0 * G->numberOfEdges();
-    }
-    else {
-        volG = 2.0 * G->totalEdgeWeight();
-    }
+    double volG = GraphTools::volume(*G);
 
     if (exactly) {
         return sqrt(distances[u][v] * volG);
@@ -188,13 +184,12 @@ double CommuteTimeDistance::runSinglePair(node u, node v) {
     lamg.solve(rhs, solution);
     double diff = solution[u] - solution[v];
     dist = fabs(diff);
-    return sqrt(dist* G->numberOfEdges());
+    return sqrt(dist * GraphTools::volume(*G));
 }
 
 double CommuteTimeDistance::runSingleSource(node u) {
     count n = G->numberOfNodes();
     double dist = 0.0;
-    double sum = 0.0;
     Vector zeroVector(n, 0.0);
     // set up solution vector and status
     std::vector<Vector> rhs(n, Vector(n));
@@ -216,11 +211,10 @@ double CommuteTimeDistance::runSingleSource(node u) {
     G->forNodes([&](node i){
         if (i != u) {
             double diff = solution[i][u] - solution[i][i];
-            dist = fabs(diff);
-            sum += sqrt(dist);
+            dist += diff * diff;
         }
     });
-    return sum * sqrt(G->numberOfEdges());
+    return sqrt(dist * GraphTools::volume(*G));
 }
 
 }

networkit/cpp/graph/GraphTools.cpp

@@ -200,6 +200,17 @@ double density(const Graph &G) noexcept {
     return m / (n * (n - 1));
 }
 
+double volume(const Graph &G) {
+    if (G.isDirected())
+        throw std::runtime_error("Volume computation is only supported for undirected graphs.");
+
+    if (!G.isWeighted()) {
+        return 2.0 * G.numberOfEdges();
+    } else {
+        return 2.0 * G.totalEdgeWeight();
+    }
+}
+
 Graph copyNodes(const Graph &G) {
     Graph C(G.upperNodeIdBound(), G.isWeighted(), G.isDirected());
     for (node u = 0; u < G.upperNodeIdBound(); ++u) {