Implementation of algorithms on cographs #23

aSmallTyphoon · 2024-04-20T08:46:03Z

No description provided.

krzysztof-turowski

Thank you very much for your work - I have a lot of conceptual comments for now (especially since the proper logic seems to be good) and some stylistic measure, please let me know if everything is clear and sound.

krzysztof-turowski · 2024-05-04T07:42:14Z

benchmark/benchmarkCograph_rec.cpp

+    std::ifstream fin("../../input/cographConnected11.g6");
+    //std::ifstream fin("../../input/graph8c.g6");


Please just provide the name of the file in cin (as in benchmarkPerfectGraphs.cpp)

krzysztof-turowski · 2024-05-04T07:44:41Z

benchmark/benchmarkCograph_rec.cpp

+    std::string types[] = {
+            "UNKNOWN",
+            "COGRAPH",
+            "IS_NOT_COGRAPH"


Suggestion: "NOT_COGRAPH"

krzysztof-turowski · 2024-05-04T07:45:33Z

benchmark/benchmarkCograph_rec.cpp

+            if (recognize.cotree->GetMaxIndependetSetSize() != recognize.cotree->BruteForceIndependetSetSize()) {
+                std::cout << "error1" << std::endl;
+            }
+
+            if (recognize.cotree->GetMaxCliqueSize() != recognize.cotree->BruteForceCliqueSize()) {
+                std::cout << "error2" << std::endl;
+            }
+
+            recognize.cotree->Coloring();
+            if (!recognize.cotree->CheckColoring()) {
+                std::cout << "error3" << std::endl;
+            }


Eventually please turn these checks to asserts

krzysztof-turowski · 2024-05-04T07:47:30Z

benchmark/benchmarkPerfectGraphs.cpp

@@ -20,6 +20,7 @@ int main() {
    };

    while (true) {
+        std::ifstream fin("../../input/");


Please remove this line

krzysztof-turowski · 2024-05-04T07:51:57Z

cpp/cograph_rec/cograph/IndependentSet.cpp

+#include "cograph_rec/Cotree.hpp"
+
+namespace Koala {
+    long long Cotree::MaxIndependetSetSize(long long n, long long v) {


Please go everywhere with NetworKit::count instead of long long unless you have a good reason to enforce this.

krzysztof-turowski · 2024-05-04T08:16:07Z

include/cograph_rec/Cotree.hpp

+    class Cotree {
+    public:
+        NetworKit::Graph *graph;
+        std::vector<long long> left_son, right_son, parent, type, size, color, number_of_colors;


It's probably better to create an auxiliary structure CoNode with fields left, right, parent, type and define std::vector here (if it's a vector of structs, they are also allocated as chunks of continuous memory, unlike vector of pointers, so it's fast)

krzysztof-turowski · 2024-05-04T08:18:48Z

cpp/cograph_rec/CMakeLists.txt

@@ -0,0 +1,10 @@
+koala_add_module(cograph_rec


Eventually you should split this across multiple modules: recognition, coloring, independent set. Please look at the PerfectGraphColoring.hpp/cpp for the scheme.

krzysztof-turowski · 2024-05-04T08:20:27Z

include/cograph_rec/Cotree.hpp

+
+        long long MaxIndependetSetSize(long long n, long long v);
+
+        long long GetMaxIndependetSetSize();


This should not be CoTree methods, rather separate classes inheriting from NetworKit::Algorithms in their proper places, please check the existing algorithms for the scheme.

krzysztof-turowski · 2024-05-04T08:20:47Z

cpp/cograph_rec/cograph/Coloring.cpp

+        for (int i = 0; i < 2 * n + 2; i++) {
+            color.push_back(0);
+            number_of_colors.push_back(0);
+        }


Again, vector.resize is the way

krzysztof-turowski · 2024-05-04T08:21:47Z

cpp/cograph_rec/cograph/Coloring.cpp

+        return color[i];
+    }
+
+    bool Cotree::CheckColoring() {


If you implement a proper CographColoring class and inherit from VertexColoringAlgorithm class, then you'll get check method for free.

krzysztof-turowski

Thank you very much, again quite a lot of conceptual changes suggested - please reach me on- or offline for further clarifications, if necessary.

krzysztof-turowski · 2024-05-20T18:41:28Z

cpp/CMakeLists.txt

@@ -8,3 +8,5 @@ add_subdirectory(mst)
 add_subdirectory(recognition)
 add_subdirectory(set_cover)
 add_subdirectory(traversal)
+add_subdirectory(max_clique)
+add_subdirectory(structures)


(1) Please add empty line at the end
(2) Please sort the subdirs alphabetically

krzysztof-turowski · 2024-05-20T18:42:32Z

cpp/coloring/CographVertexColoring.cpp

+
+namespace Koala {
+    void CographVertexColoring::SubtreeColors(NetworKit::count v) {
+        if (recognition->cotree->nodes[v].left_son != -1) {


Please insert as a first line

auto &cotree_v = recognition->cotree->nodes[v];

It would drastically shorten the subsequent code

krzysztof-turowski · 2024-05-20T18:44:00Z

cpp/coloring/CographVertexColoring.cpp

+    NetworKit::count CographVertexColoring::GetColor(NetworKit::count i) {
+        if (recognition->cotree->prepared == false) {
+            recognition->cotree->BuildTree();
+        }
+        return color[i];
+    }


I'd replace it with getColoring returning a map (as e.g. in VertexColoring) + this map would be built during run.

krzysztof-turowski · 2024-05-20T18:44:20Z

cpp/coloring/CographVertexColoring.cpp

+        }
+        return true;
+    }
+}


Please fix empty line at the end

krzysztof-turowski · 2024-05-20T18:45:52Z

cpp/coloring/CographVertexColoring.cpp

+    }
+
+    bool CographVertexColoring::CheckColoring() {
+        NetworKit::count n = graph->numberOfNodes(), i, j;


Please don't use (unsafe) C-style way of defining temporary variables unitinialized and outside of for loop. That's just really bad idiom, unless absolutely necessary.

krzysztof-turowski · 2024-05-20T19:08:38Z

cpp/recognition/cograph/FactorizingPermutation.cpp

+                    std::cout << y->num << " ";
+                }
+            }
+            std::cout << " pivot=";


Well, I guess these shouldn't be left here - and in few other places.

krzysztof-turowski · 2024-05-20T19:10:49Z

cpp/independent_set/CographIndependentSet.cpp

+        if (cotree->prepared == false) {
+            cotree->BuildTree();
+        }
+        independet_set_size = recurse_run(n, n);


Do you always pass n as the first parameter? If so, why can't you just get it via G.numberOfNodes()?

krzysztof-turowski · 2024-05-20T19:11:20Z

cpp/max_clique/MaxClique.cpp

+#include "recognition/CographAlg.hpp"
+
+namespace Koala {
+    NetworKit::count MaxClique::recurse_run(NetworKit::count n, NetworKit::count v) {


To be honest, you never use the first parameter in the method

krzysztof-turowski · 2024-05-20T19:12:01Z

cpp/pathwidth/Pathwidth.cpp

+    }
+
+    NetworKit::count Pathwidth::pathwidth(NetworKit::count n, NetworKit::count v) {
+        if (recognition->cotree->nodes[v].type == 2) {


Again, extracting recognition->cotree->nodes[v] to an autoref variable would simplify your code a lot.

krzysztof-turowski · 2024-05-20T19:12:57Z

cpp/pathwidth/Pathwidth.cpp

+        if (recognition->cotree->prepared == false) {
+            recognition->cotree->BuildTree();
+        }


Why do you check if cotree is prepared if you already forced computing it using recognition->run()?

aSmallTyphoon added 12 commits April 16, 2024 18:09

first version

dea18e0

second version

2ff23be

2.1 version

4c075f9

2.1 version

e017ca1

2.1 version

215537a

2.1 version

4b4d327

2.1 version

af5cdcb

2.1 version

30048da

2.2 version

cdc1b0d

2.2 version

42902b0

2.2 version

7cfe94e

2.2 version

c7fc176

krzysztof-turowski requested changes May 4, 2024

View reviewed changes

krzysztof-turowski added the algorithms New feature or request label May 7, 2024

krzysztof-turowski assigned aSmallTyphoon May 7, 2024

aSmallTyphoon added 2 commits May 15, 2024 20:36

3 version

e306653

3.1 version

98df863

krzysztof-turowski requested changes May 20, 2024

View reviewed changes

aSmallTyphoon added 5 commits May 23, 2024 21:12

4 version

883d29e

4.1 version

560e91f

4.2 version

f8612b6

5 version

05f2a3f

5.1 version

1626835

aSmallTyphoon changed the title ~~first version~~ Implementation of algorithms on cographs Jul 2, 2024

6 version

c831fcd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementation of algorithms on cographs #23

Implementation of algorithms on cographs #23

aSmallTyphoon commented Apr 20, 2024

krzysztof-turowski left a comment

krzysztof-turowski May 4, 2024

krzysztof-turowski May 4, 2024

krzysztof-turowski May 4, 2024

krzysztof-turowski May 4, 2024

krzysztof-turowski May 4, 2024

krzysztof-turowski May 4, 2024

krzysztof-turowski May 4, 2024

krzysztof-turowski May 4, 2024

krzysztof-turowski May 4, 2024

krzysztof-turowski May 4, 2024

krzysztof-turowski left a comment

krzysztof-turowski May 20, 2024

krzysztof-turowski May 20, 2024

krzysztof-turowski May 20, 2024

krzysztof-turowski May 20, 2024

krzysztof-turowski May 20, 2024

krzysztof-turowski May 20, 2024

krzysztof-turowski May 20, 2024

krzysztof-turowski May 20, 2024

krzysztof-turowski May 20, 2024

krzysztof-turowski May 20, 2024

		std::ifstream fin("../../input/cographConnected11.g6");
		//std::ifstream fin("../../input/graph8c.g6");


		long long MaxIndependetSetSize(long long n, long long v);

		long long GetMaxIndependetSetSize();

Implementation of algorithms on cographs #23

Are you sure you want to change the base?

Implementation of algorithms on cographs #23

Conversation

aSmallTyphoon commented Apr 20, 2024

krzysztof-turowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzysztof-turowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment