From 6489b99fc6936dd2967772faf3318f3fef42a805 Mon Sep 17 00:00:00 2001 From: Moritz Sallermann Date: Sun, 1 Oct 2023 11:40:20 +0000 Subject: [PATCH 1/7] Added a unit test for DeGroot model --- test/test_deGroot.cpp | 44 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 test/test_deGroot.cpp diff --git a/test/test_deGroot.cpp b/test/test_deGroot.cpp new file mode 100644 index 0000000..843ecf4 --- /dev/null +++ b/test/test_deGroot.cpp @@ -0,0 +1,44 @@ +#include +#include + +#include "models/DeGroot.hpp" +#include "network.hpp" +#include + +TEST_CASE( "Test the DeGroot Model Symmetric", "[DeGroot]" ) +{ + using namespace Seldon; + using namespace Catch::Matchers; + + size_t n_agents = 2; + auto neighbour_list = std::vector>{ + { 1, 0 }, + { 0, 1 }, + }; + + auto weight_list = std::vector>{ + { 0.2, 0.8 }, + { 0.2, 0.8 }, + }; + + auto gen = std::mt19937(); + auto network = Network( std::move( neighbour_list ), std::move( weight_list ), gen ); + auto model = DeGrootModel( n_agents, network ); + + model.convergence_tol = 1e-6; + model.max_iterations = 100; + model.agents[0].opinion = 0.0; + model.agents[1].opinion = 1.0; + + do + { + model.iteration(); + } while( !model.finished() ); + + fmt::print( "N_iterations = {} (with convergence_tol {})\n", model.n_iterations, model.convergence_tol ); + for( int i = 0; i < n_agents; i++ ) + { + fmt::print( "Opinion {} = {}\n", i, model.agents[i].opinion ); + REQUIRE_THAT( model.agents[i].opinion, WithinAbs( 0.5, model.convergence_tol * 10.0 ) ); + } +} \ No newline at end of file From 4edcf9ad9d42e57b08beea80b222c1bccf9d86b2 Mon Sep 17 00:00:00 2001 From: Moritz Sallermann Date: Sun, 1 Oct 2023 11:40:32 +0000 Subject: [PATCH 2/7] Small improvements for test_tarjan --- test/test_tarjan.cpp | 71 +++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 41 deletions(-) diff --git a/test/test_tarjan.cpp b/test/test_tarjan.cpp index 9e38791..c7fb551 100644 --- a/test/test_tarjan.cpp +++ b/test/test_tarjan.cpp @@ -1,6 +1,9 @@ #include #include "connectivity.hpp" +#include +#include +#include #include // Create the vector of vectors containing the neighbour indices @@ -24,53 +27,39 @@ // | J +<--+ I | // +---+ +---+ -void create_neighbour_list( std::vector> & neighbour_list ) { - - auto buffer = std::vector(); // contains indices - // // --- Create the list - buffer.resize( 1, 1 ); - neighbour_list.push_back( buffer ); // A or 0 - buffer.resize( 2 ); - buffer[0] = 2; - buffer[1] = 3; - neighbour_list.push_back( buffer ); // B or 1 - buffer.resize( 1 ); - buffer[0] = 0; - neighbour_list.push_back( buffer ); // C or 2 - buffer[0] = 4; - neighbour_list.push_back( buffer ); // D or 3 - buffer[0] = 5; - neighbour_list.push_back( buffer ); // E or 4 - buffer[0] = 4; - neighbour_list.push_back( buffer ); // F or 5 - // Second tree (G,H,I,J) - buffer.resize( 2 ); - buffer[0] = 4; - buffer[1] = 7; - neighbour_list.push_back( buffer ); // G or 6 - buffer[0] = 5; - buffer[1] = 8; - neighbour_list.push_back( buffer ); // H or 7 - buffer.resize( 1 ); - buffer[0] = 9; - neighbour_list.push_back( buffer ); // I or 8 - buffer.resize( 2 ); - buffer[0] = 6; - buffer[1] = 7; - neighbour_list.push_back( buffer ); // J or 9 -} +TEST_CASE( "Test for Tarjan's algorithm for strongly connected networks", "[tarjan]" ) +{ -TEST_CASE( "Test for Tarjan's algorithm for strongly connected networks", "[tarjan]" ) { - - std::vector> neighbour_list; - create_neighbour_list( neighbour_list ); + // clang-format off + std::vector> neighbour_list = std::vector>{ + {1}, + {2,3}, + {0}, + {4}, + {5}, + {4}, + {4,7}, + {5,8}, + {9}, + {6,7} + }; + // clang-format on auto tarjan_scc = Seldon::TarjanConnectivityAlgo( neighbour_list ); // List of SCC // [[5, 4], [3], [2, 1, 0], [9, 8, 7, 6]] + fmt::print( "SCC = {}\n", tarjan_scc.scc_list ); - // There should be 4 strongly connected components + std::set> expected_scc{ { 5, 4 }, { 3 }, { 2, 1, 0 }, { 9, 8, 7, 6 } }; + + for( const auto & scc : tarjan_scc.scc_list ) + { + std::set temp_set; + temp_set.insert( scc.begin(), scc.end() ); + REQUIRE( expected_scc.contains( temp_set ) ); + } + + // There should be 4 strongly connected components REQUIRE( tarjan_scc.scc_list.size() == 4 ); - } \ No newline at end of file From 8cafebb126b748bd258a91f1b29953e691bacfd9 Mon Sep 17 00:00:00 2001 From: Moritz Sallermann Date: Sun, 1 Oct 2023 11:42:14 +0000 Subject: [PATCH 3/7] CI: added unit tests --- .github/workflows/ci.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf260ca..e44e189 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,4 +29,9 @@ jobs: shell: pixi run bash {0} run: | meson setup build - meson compile -C build \ No newline at end of file + meson compile -C build + + - name: Test with meson + shell: pixi run bash {0} + run: | + meson test -C build \ No newline at end of file From ed43f7cfca97cc621ab09fb1d22277faa2c59929 Mon Sep 17 00:00:00 2001 From: Moritz Sallermann Date: Sun, 1 Oct 2023 11:43:59 +0000 Subject: [PATCH 4/7] added test_deGroot in meson.build --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index e336f3a..ff6d5c2 100644 --- a/meson.build +++ b/meson.build @@ -22,6 +22,7 @@ exe = executable('seldon', sources_seldon + 'src/main.cpp', tests = [ ['Test Tarjan', 'test/test_tarjan.cpp'], + ['Test DeGroot', 'test/test_deGroot.cpp'] ] Catch2 = dependency('Catch2', method : 'cmake', modules : ['Catch2::Catch2WithMain', 'Catch2::Catch2']) From 74248ddc32ef7c17d63c91f53fd6b52e8accef98 Mon Sep 17 00:00:00 2001 From: Moritz Sallermann Date: Sun, 1 Oct 2023 12:32:14 +0000 Subject: [PATCH 5/7] added a .clang-tidy file and a run_linter.sh script --- .clang-tidy | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++ run_linter.sh | 4 ++ 2 files changed, 156 insertions(+) create mode 100644 .clang-tidy create mode 100755 run_linter.sh diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..8818d13 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,152 @@ +--- +WarningsAsErrors: '' +HeaderFilterRegex: '' +AnalyzeTemporaryDtors: false +FormatStyle: file +Checks: > + -*, + clang-diagnostic-*, + bugprone-argument-comment, + bugprone-assert-side-effect, + bugprone-bad-signal-to-kill-thread, + bugprone-bool-pointer-implicit-conversion, + bugprone-branch-clone, + bugprone-copy-constructor-init, + bugprone-dangling-handle, + bugprone-exception-escape, + bugprone-fold-init-type, + bugprone-forward-declaration-namespace, + bugprone-forwarding-reference-overload, + bugprone-implicit-widening-of-multiplication-result, + bugprone-inaccurate-erase, + bugprone-incorrect-roundings, + bugprone-infinite-loop, + bugprone-integer-division, + bugprone-lambda-function-name, + bugprone-macro-parentheses, + bugprone-macro-repeated-side-effects, + bugprone-misplaced-operator-in-strlen-in-alloc, + bugprone-misplaced-pointer-arithmetic-in-alloc, + bugprone-misplaced-widening-cast, + bugprone-move-forwarding-reference, + bugprone-multiple-statement-macro, + bugprone-not-null-terminated-result, + bugprone-parent-virtual-call, + bugprone-posix-return, + bugprone-redundant-branch-condition, + bugprone-reserved-identifier, + bugprone-signal-handler, + bugprone-signed-char-misuse, + bugprone-sizeof-container, + bugprone-sizeof-expression, + bugprone-spuriously-wake-up-functions, + bugprone-string-constructor, + bugprone-string-integer-assignment, + bugprone-string-literal-with-embedded-nul, + bugprone-suspicious-enum-usage, + bugprone-suspicious-include, + bugprone-suspicious-memory-comparison, + bugprone-suspicious-memset-usage, + bugprone-suspicious-missing-comma, + bugprone-suspicious-semicolon, + bugprone-suspicious-string-compare, + bugprone-swapped-arguments, + bugprone-terminating-continue, + bugprone-throw-keyword-missing, + bugprone-too-small-loop-variable, + bugprone-undefined-memory-manipulation, + bugprone-undelegated-constructor, + bugprone-unhandled-exception-at-new, + bugprone-unhandled-self-assignment, + bugprone-unused-raii, + bugprone-unused-return-value, + bugprone-use-after-move, + bugprone-virtual-near-miss, + cert-dcl58-cpp, + cert-env33-c, + cert-err34-c, + cert-err52-cpp, + cert-err60-cpp, + cert-flp30-c, + cert-msc50-cpp, + cert-msc51-cpp, + cert-oop57-cpp, + cert-oop58-cpp, + cert-dcl59-cpp, + cppcoreguidelines-avoid-goto, + cppcoreguidelines-init-variables, + cppcoreguidelines-macro-usage, + cppcoreguidelines-no-malloc, + cppcoreguidelines-prefer-member-initializer, + cppcoreguidelines-pro-type-const-cast, + cppcoreguidelines-pro-type-cstyle-cast, + cppcoreguidelines-pro-type-member-init, + cppcoreguidelines-pro-type-static-cast-downcast, + cppcoreguidelines-slicing, + cppcoreguidelines-virtual-class-destructor, + hicpp-exception-baseclass, + hicpp-no-assembler, + hicpp-signed-bitwise, + misc-definitions-in-headers, + misc-non-copyable-objects, + misc-redundant-expression, + misc-static-assert, + misc-throw-by-value-catch-by-reference, + misc-unconventional-assign-operator, + misc-uniqueptr-reset-release, + misc-unused-alias-decls, + misc-unused-parameters, + misc-unused-using-decls, + modernize-avoid-bind, + modernize-deprecated-headers, + modernize-make-shared, + modernize-make-unique, + modernize-redundant-void-arg, + modernize-use-auto, + modernize-use-bool-literals, + modernize-use-emplace, + modernize-use-equals-default, + modernize-use-equals-delete, + modernize-use-nullptr, + modernize-use-override, + modernize-use-uncaught-exceptions, + modernize-use-using, + openmp-exception-escape, + openmp-use-default-none, + performance-for-range-copy, + performance-implicit-conversion-in-loop, + performance-inefficient-algorithm, + performance-inefficient-string-concatenation, + performance-move-const-arg, + performance-move-constructor-init, + performance-trivially-destructible, + performance-type-promotion-in-math-fn, + performance-unnecessary-copy-initialization, + performance-unnecessary-value-param, + readability-avoid-const-params-in-decls, + readability-container-size-empty, + readability-convert-member-functions-to-static, + readability-delete-null-pointer, + readability-function-cognitive-complexity, + readability-function-size, + readability-implicit-bool-conversion, + readability-inconsistent-declaration-parameter-name, + readability-make-member-function-const, + readability-misplaced-array-index, + readability-non-const-parameter, + readability-qualified-auto, + readability-redundant-access-specifiers, + readability-redundant-control-flow, + readability-redundant-declaration, + readability-redundant-preprocessor, + readability-redundant-smartptr-get, + readability-redundant-string-cstr, + readability-simplify-boolean-expr, + readability-simplify-subscript-expr, + readability-static-accessed-through-instance, + readability-string-compare, + readability-uniqueptr-delete-release, + readability-use-anyofallof +CheckOptions: + - { key: readability-function-size.StatementThreshold, value: '800' } + - { key: readability-function-size.LineThreshold, value: '200' } \ No newline at end of file diff --git a/run_linter.sh b/run_linter.sh new file mode 100755 index 0000000..1bfee74 --- /dev/null +++ b/run_linter.sh @@ -0,0 +1,4 @@ +#!/bin/sh +find ./include -regex '.*\.\(cpp\|hpp\|cc\|cxx\)' -exec clang-tidy {} \; +find ./src -regex '.*\.\(cpp\|hpp\|cc\|cxx\)' -exec clang-tidy {} \; +find ./test -regex '.*\.\(cpp\|hpp\|cc\|cxx\)' -exec clang-tidy {} \; \ No newline at end of file From 2b6e264de82b04190e00651a8093a7eaf0f266f7 Mon Sep 17 00:00:00 2001 From: Moritz Sallermann Date: Sun, 1 Oct 2023 12:32:30 +0000 Subject: [PATCH 6/7] added test directory to run_formatter.sh --- run_formatter.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/run_formatter.sh b/run_formatter.sh index 8dcbc72..522216d 100755 --- a/run_formatter.sh +++ b/run_formatter.sh @@ -1,3 +1,4 @@ #!/bin/sh find ./include -regex '.*\.\(cpp\|hpp\|cc\|cxx\)' -exec clang-format -i {} \; find ./src -regex '.*\.\(cpp\|hpp\|cc\|cxx\)' -exec clang-format -i {} \; +find ./test -regex '.*\.\(cpp\|hpp\|cc\|cxx\)' -exec clang-format -i {} \; \ No newline at end of file From 4dd901d9f054c47a17ada988beb587f228810548 Mon Sep 17 00:00:00 2001 From: Moritz Sallermann Date: Sun, 1 Oct 2023 12:34:28 +0000 Subject: [PATCH 7/7] Misc. changes after linting --- include/agent.hpp | 4 +--- include/agent_base.hpp | 2 +- include/connectivity.hpp | 2 +- include/model.hpp | 4 ++-- include/model_base.hpp | 1 + include/util/io.hpp | 7 ++++--- src/models/DeGroot.cpp | 4 ++-- src/simulation.cpp | 3 ++- test/test_deGroot.cpp | 4 ++-- 9 files changed, 16 insertions(+), 15 deletions(-) diff --git a/include/agent.hpp b/include/agent.hpp index c06eab6..215dec7 100644 --- a/include/agent.hpp +++ b/include/agent.hpp @@ -16,12 +16,10 @@ class Agent : public AgentBase void from_string( const std::string & str ); - virtual std::string to_string() const override + std::string to_string() const override { return fmt::format( "{}", opinion ); } - - ~Agent() = default; }; template<> diff --git a/include/agent_base.hpp b/include/agent_base.hpp index d591ad3..88c23b0 100644 --- a/include/agent_base.hpp +++ b/include/agent_base.hpp @@ -9,7 +9,7 @@ class AgentBase { public: virtual std::string to_string() const = 0; - + virtual ~AgentBase() = default; // TODO: eventually a from_string might also be needed // virtual void from_string() = 0; }; diff --git a/include/connectivity.hpp b/include/connectivity.hpp index e9ee5f2..1af9c8d 100644 --- a/include/connectivity.hpp +++ b/include/connectivity.hpp @@ -85,7 +85,7 @@ class TarjanConnectivityAlgo if( lowest[v] == num[v] ) { scc.resize( 0 ); - size_t scc_vertex; + size_t scc_vertex = 0; // Pop the stack scc_vertex = stack.back(); stack.pop_back(); diff --git a/include/model.hpp b/include/model.hpp index 113d875..5b44e5e 100644 --- a/include/model.hpp +++ b/include/model.hpp @@ -57,9 +57,9 @@ class Model : public ModelBase } } - virtual void iteration() override = 0; + void iteration() override = 0; - virtual bool finished() override + bool finished() override { if( max_iterations.has_value() ) { diff --git a/include/model_base.hpp b/include/model_base.hpp index cb61a05..86159ae 100644 --- a/include/model_base.hpp +++ b/include/model_base.hpp @@ -16,6 +16,7 @@ class ModelBase virtual AgentBase * get_agent( int idx ) = 0; // Use this to get an abstract representation of the agent at idx virtual void iteration() = 0; virtual bool finished() = 0; + virtual ~ModelBase() = default; }; } // namespace Seldon \ No newline at end of file diff --git a/include/util/io.hpp b/include/util/io.hpp index 92dc369..791f47f 100644 --- a/include/util/io.hpp +++ b/include/util/io.hpp @@ -12,7 +12,8 @@ namespace Seldon namespace IO { -inline void network_to_dot_file( const Network & network, std::string file_path ) + +inline void network_to_dot_file( const Network & network, const std::string & file_path ) { std::fstream fs; fs.open( file_path, std::fstream::in | std::fstream::out | std::fstream::trunc ); @@ -38,7 +39,7 @@ inline void network_to_dot_file( const Network & network, std::string file_path fs.close(); } -inline void opinions_to_file( Simulation & simulation, std::string file_path ) +inline void opinions_to_file( Simulation & simulation, const std::string & file_path ) { std::fstream fs; fs.open( file_path, std::fstream::in | std::fstream::out | std::fstream::trunc ); @@ -56,7 +57,7 @@ inline void opinions_to_file( Simulation & simulation, std::string file_path ) fs.close(); } -inline void network_to_file( Simulation & simulation, std::string file_path ) +inline void network_to_file( Simulation & simulation, const std::string & file_path ) { std::fstream fs; fs.open( file_path, std::fstream::in | std::fstream::out | std::fstream::trunc ); diff --git a/src/models/DeGroot.cpp b/src/models/DeGroot.cpp index 15b7f87..153d6e9 100644 --- a/src/models/DeGroot.cpp +++ b/src/models/DeGroot.cpp @@ -17,8 +17,8 @@ void Seldon::DeGrootModel::iteration() auto neighbour_buffer = std::vector(); auto weight_buffer = std::vector(); - size_t j_index; - double weight; + size_t j_index = 0; + double weight = 0.0; for( size_t i = 0; i < agents.size(); i++ ) { diff --git a/src/simulation.cpp b/src/simulation.cpp index acc2c13..93801c9 100644 --- a/src/simulation.cpp +++ b/src/simulation.cpp @@ -7,6 +7,7 @@ #include #include #include +#include enum class ModelType : unsigned int { @@ -42,7 +43,7 @@ Seldon::Simulation::Simulation( // Check if 'model' is one of the allowed values auto model_string = model_opt.value(); - if( !allowed_models.count( model_string ) ) + if( !allowed_models.contains( model_string ) ) { throw std::runtime_error( fmt::format( "Unknown model type: '{}'!", model_string ) ); } diff --git a/test/test_deGroot.cpp b/test/test_deGroot.cpp index 843ecf4..65128c0 100644 --- a/test/test_deGroot.cpp +++ b/test/test_deGroot.cpp @@ -22,7 +22,7 @@ TEST_CASE( "Test the DeGroot Model Symmetric", "[DeGroot]" ) }; auto gen = std::mt19937(); - auto network = Network( std::move( neighbour_list ), std::move( weight_list ), gen ); + auto network = Network( std::move( neighbour_list ), std::move( weight_list ) ); auto model = DeGrootModel( n_agents, network ); model.convergence_tol = 1e-6; @@ -36,7 +36,7 @@ TEST_CASE( "Test the DeGroot Model Symmetric", "[DeGroot]" ) } while( !model.finished() ); fmt::print( "N_iterations = {} (with convergence_tol {})\n", model.n_iterations, model.convergence_tol ); - for( int i = 0; i < n_agents; i++ ) + for( size_t i = 0; i < n_agents; i++ ) { fmt::print( "Opinion {} = {}\n", i, model.agents[i].opinion ); REQUIRE_THAT( model.agents[i].opinion, WithinAbs( 0.5, model.convergence_tol * 10.0 ) );