From aa446dbcd9d349d54ff8bddfcb57d16c59ad501f Mon Sep 17 00:00:00 2001 From: Thorsten Hater <24411438+thorstenhater@users.noreply.github.com> Date: Mon, 9 Sep 2024 15:33:15 +0200 Subject: [PATCH 1/7] Fix GCC's paranoia and use iota --- arbor/tree.cpp | 28 +++++++++++----------------- arbor/tree.hpp | 4 ---- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/arbor/tree.cpp b/arbor/tree.cpp index 3ae342dff0..1407303575 100644 --- a/arbor/tree.cpp +++ b/arbor/tree.cpp @@ -29,7 +29,7 @@ tree::tree(std::vector parent_index) { parents_[0] = no_parent; // compute offsets into children_ array - arb::util::make_partition(child_index_, child_count(parents_)); + util::make_partition(child_index_, child_count(parents_)); std::vector pos(parents_.size(), 0); for (auto i = 1u; i < parents_.size(); ++i) { @@ -199,11 +199,9 @@ tree::iarray tree::select_new_root(int_type root) { } // maps new indices to old indices - iarray indices (num_nodes); - // fill array with indices - for (auto i: make_span(num_nodes)) { - indices[i] = i; - } + iarray indices(num_nodes); + std::iota(indices.begin(), indices.end(), 0); + // perform sort by depth index to get the permutation std::sort(indices.begin(), indices.end(), [&](auto i, auto j){ if (reduced_depth[i] != reduced_depth[j]) { @@ -214,16 +212,12 @@ tree::iarray tree::select_new_root(int_type root) { } return depth[i] < depth[j]; }); - // maps old indices to new indices - iarray indices_inv (num_nodes); - // fill array with indices - for (auto i: make_span(num_nodes)) { - indices_inv[i] = i; - } - // perform sort - std::sort(indices_inv.begin(), indices_inv.end(), [&](auto i, auto j){ - return indices[i] < indices[j]; - }); + + // inverse permutation + iarray indices_inv(num_nodes, 0); + std::iota(indices_inv.begin(), indices_inv.end(), 0); + std::sort(indices_inv.begin(), indices_inv.end(), + [&](auto i, auto j){ return indices[i] < indices[j]; }); // translate the parent vetor to new indices for (auto i: make_span(num_nodes)) { @@ -241,7 +235,7 @@ tree::iarray tree::select_new_root(int_type root) { // recompute the children array memory::copy(new_parents, parents_); - arb::util::make_partition(child_index_, child_count(parents_)); + util::make_partition(child_index_, child_count(parents_)); std::vector pos(parents_.size(), 0); for (auto i = 1u; i < parents_.size(); ++i) { diff --git a/arbor/tree.hpp b/arbor/tree.hpp index 137598318a..75b6fb2f8f 100644 --- a/arbor/tree.hpp +++ b/arbor/tree.hpp @@ -2,16 +2,12 @@ #include #include -#include -#include #include #include #include -#include "memory/memory.hpp" #include "util/rangeutil.hpp" -#include "util/span.hpp" namespace arb { From 51deda05503bd38569e1194c284a64363ae91e5d Mon Sep 17 00:00:00 2001 From: Thorsten Hater <24411438+thorstenhater@users.noreply.github.com> Date: Tue, 10 Sep 2024 08:23:56 +0200 Subject: [PATCH 2/7] More paranoid GCC. --- arbor/util/meta.hpp | 1 - arbor/util/range.hpp | 16 ++++--------- arbor/util/rangeutil.hpp | 1 - .../test_distributed_for_each.cpp | 13 +++++------ .../test_network_generation.cpp | 8 +++---- test/unit/test_range.cpp | 23 +++++-------------- test/unit/test_simd.cpp | 4 ++-- 7 files changed, 23 insertions(+), 43 deletions(-) diff --git a/arbor/util/meta.hpp b/arbor/util/meta.hpp index d9f30584fd..ed0055f088 100644 --- a/arbor/util/meta.hpp +++ b/arbor/util/meta.hpp @@ -5,7 +5,6 @@ #include #include #include -#include #include #include diff --git a/arbor/util/range.hpp b/arbor/util/range.hpp index 3a7257b7c1..5f01b6f4dd 100644 --- a/arbor/util/range.hpp +++ b/arbor/util/range.hpp @@ -156,12 +156,9 @@ range range_n(T t, size_t n) { template auto canonical_view(Seq&& s) { - using std::begin; - using std::end; - - return make_range( - make_sentinel_iterator(begin(s), end(s)), - make_sentinel_end(begin(s), end(s))); + auto b = std::begin(s); + auto e = std::end(s); + return make_range(make_sentinel_iterator(b, e), make_sentinel_end(b, e)); } // Strictly evaluate end point in sentinel-terminated range and present as a range over @@ -169,11 +166,8 @@ auto canonical_view(Seq&& s) { template auto strict_view(Seq&& s) { - using std::begin; - using std::end; - - auto b = begin(s); - auto e = end(s); + auto b = std::begin(s); + auto e = std::end(s); return make_range(b, b==e? b: std::next(util::upto(b, e))); } diff --git a/arbor/util/rangeutil.hpp b/arbor/util/rangeutil.hpp index 312bd55877..045ea79d7d 100644 --- a/arbor/util/rangeutil.hpp +++ b/arbor/util/rangeutil.hpp @@ -7,7 +7,6 @@ #include #include -#include #include #include #include diff --git a/test/unit-distributed/test_distributed_for_each.cpp b/test/unit-distributed/test_distributed_for_each.cpp index 5c545e8c53..b9ca83b228 100644 --- a/test/unit-distributed/test_distributed_for_each.cpp +++ b/test/unit-distributed/test_distributed_for_each.cpp @@ -40,10 +40,9 @@ TEST(distributed_for_each, one_zero) { for (int i = 0; i < rank; ++i) { data.push_back(rank); } auto sample = [&](const util::range& range) { - const auto origin_rank = range.empty() ? 0 : range.front(); - + std::size_t origin_rank = range.empty() ? 0 : range.front(); EXPECT_EQ(origin_rank, range.size()); - for (const auto& value: range) { EXPECT_EQ(value, origin_rank); } + for (std::size_t value: range) { EXPECT_EQ(value, origin_rank); } ++call_count; }; @@ -71,14 +70,14 @@ TEST(distributed_for_each, multiple) { auto sample = [&](const util::range& range_1, const util::range& range_2, const util::range*>& range_3) { - const auto origin_rank = range_1.empty() ? 0 : range_1.front(); + std::size_t origin_rank = range_1.empty() ? 0 : range_1.front(); EXPECT_EQ(origin_rank + 1, range_1.size()); EXPECT_EQ(range_2.size(), 2 * range_1.size()); EXPECT_EQ(range_3.size(), 3 * range_1.size()); - for (const auto& value: range_1) { EXPECT_EQ(value, origin_rank); } - for (const auto& value: range_2) { EXPECT_EQ(value, double(origin_rank)); } - for (const auto& value: range_3) { EXPECT_EQ(value, std::complex(origin_rank)); } + for (std::size_t value: range_1) { EXPECT_EQ(value, origin_rank); } + for (auto value: range_2) { EXPECT_EQ(value, double(origin_rank)); } + for (auto value: range_3) { EXPECT_EQ(value, std::complex(origin_rank)); } ++call_count; }; diff --git a/test/unit-distributed/test_network_generation.cpp b/test/unit-distributed/test_network_generation.cpp index f7e3b55fe3..1a6e094742 100644 --- a/test/unit-distributed/test_network_generation.cpp +++ b/test/unit-distributed/test_network_generation.cpp @@ -125,8 +125,8 @@ TEST(network_generation, all) { } for (const auto& group: decomp.groups()) { - const auto num_dest = group.kind == cell_kind::spike_source ? 0 : 1; - for (const auto gid: group.gids) { + std::size_t num_dest = group.kind == cell_kind::spike_source ? 0 : 1; + for (std::size_t gid: group.gids) { EXPECT_EQ(connections_by_dest[gid].size(), num_cells * num_dest); } } @@ -141,7 +141,7 @@ TEST(network_generation, cable_only) { const auto weight = 2.0; const auto delay = 3.0; - const auto num_cells = 3 * num_ranks; + const std::size_t num_cells = 3 * num_ranks; auto rec = network_test_recipe(num_cells, selection, weight, delay); @@ -161,7 +161,7 @@ TEST(network_generation, cable_only) { for (const auto gid: group.gids) { // Only one third is a cable cell EXPECT_EQ(connections_by_dest[gid].size(), - group.kind == cell_kind::cable ? num_cells / 3 : 0); + group.kind == cell_kind::cable ? num_cells / 3 : 0); } } } diff --git a/test/unit/test_range.cpp b/test/unit/test_range.cpp index 02e889fc0c..29bd74779e 100644 --- a/test/unit/test_range.cpp +++ b/test/unit/test_range.cpp @@ -14,7 +14,6 @@ #include "util/meta.hpp" #include "util/range.hpp" #include "util/rangeutil.hpp" -#include "util/sentinel.hpp" #include "util/transform.hpp" #include "common.hpp" @@ -444,33 +443,23 @@ struct foo { TEST(range, sort) { char cstr[] = "howdy"; - auto cstr_range = util::make_range(std::begin(cstr), null_terminated); - - // Alas, no forward_iterator sort yet, so make a strict (non-sentinel) - // range to sort on below - // simple sort - util::sort(util::strict_view(cstr_range)); + util::sort(util::make_range(std::begin(cstr), null_terminated)); + // std::sort(view.begin(), view.end()); EXPECT_EQ("dhowy"s, cstr); // reverse sort by transform c to -c - util::sort_by(util::strict_view(cstr_range), [](char c) { return -c; }); + util::sort_by(util::make_range(std::begin(cstr), null_terminated), + [](char c) { return -c; }); EXPECT_EQ("ywohd"s, cstr); // stable sort: move capitals to front, numbers to back - auto rank = [](char c) { - return std::isupper(c)? 0: std::isdigit(c)? 2: 1; - }; - char mixed[] = "t5hH4E3erLL2e1O"; - auto mixed_range = util::make_range(std::begin(mixed), null_terminated); - - util::stable_sort_by(util::strict_view(mixed_range), rank); + util::stable_sort_by(util::make_range(std::begin(mixed), null_terminated), + [](char c) { return std::isupper(c)? 0: std::isdigit(c)? 2: 1; }); EXPECT_EQ("HELLOthere54321"s, mixed); - // sort with user-provided less comparison function - std::vector X = {{0, 5}, {1, 4}, {2, 3}, {3, 2}, {4, 1}, {5, 0}}; util::sort(X, [](const foo& l, const foo& r) {return l.yb; From 1dace6e61cf4a6ec2011df5423042a286f946dcf Mon Sep 17 00:00:00 2001 From: Thorsten Hater <24411438+thorstenhater@users.noreply.github.com> Date: Tue, 10 Sep 2024 08:30:25 +0200 Subject: [PATCH 3/7] Clean-up. --- test/unit/test_range.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/test_range.cpp b/test/unit/test_range.cpp index 29bd74779e..a5325dd3a0 100644 --- a/test/unit/test_range.cpp +++ b/test/unit/test_range.cpp @@ -445,7 +445,6 @@ TEST(range, sort) { // simple sort util::sort(util::make_range(std::begin(cstr), null_terminated)); - // std::sort(view.begin(), view.end()); EXPECT_EQ("dhowy"s, cstr); // reverse sort by transform c to -c From d205d8efb8d14de99ab8fe47e567e29fb7c2f451 Mon Sep 17 00:00:00 2001 From: Thorsten Hater <24411438+thorstenhater@users.noreply.github.com> Date: Tue, 10 Sep 2024 08:53:32 +0200 Subject: [PATCH 4/7] Weird GCC ./. Clang issue --- test/unit/common.hpp | 1 + test/unit/test_range.cpp | 52 +++++++++++++++++++++------------------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/test/unit/common.hpp b/test/unit/common.hpp index e02b9df20a..23a08da975 100644 --- a/test/unit/common.hpp +++ b/test/unit/common.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include diff --git a/test/unit/test_range.cpp b/test/unit/test_range.cpp index a5325dd3a0..e096a2e4a1 100644 --- a/test/unit/test_range.cpp +++ b/test/unit/test_range.cpp @@ -441,30 +441,34 @@ struct foo { }; TEST(range, sort) { - char cstr[] = "howdy"; - - // simple sort - util::sort(util::make_range(std::begin(cstr), null_terminated)); - EXPECT_EQ("dhowy"s, cstr); - - // reverse sort by transform c to -c - util::sort_by(util::make_range(std::begin(cstr), null_terminated), - [](char c) { return -c; }); - EXPECT_EQ("ywohd"s, cstr); - - // stable sort: move capitals to front, numbers to back - char mixed[] = "t5hH4E3erLL2e1O"; - util::stable_sort_by(util::make_range(std::begin(mixed), null_terminated), - [](char c) { return std::isupper(c)? 0: std::isdigit(c)? 2: 1; }); - EXPECT_EQ("HELLOthere54321"s, mixed); - - // sort with user-provided less comparison function - std::vector X = {{0, 5}, {1, 4}, {2, 3}, {3, 2}, {4, 1}, {5, 0}}; - - util::sort(X, [](const foo& l, const foo& r) {return l.y{{5, 0}, {4, 1}, {3, 2}, {2, 3}, {1, 4}, {0, 5}})); - util::sort(X, [](const foo& l, const foo& r) {return l.x{{0, 5}, {1, 4}, {2, 3}, {3, 2}, {4, 1}, {5, 0}})); + { + // simple sort + char cstr[] = "howdy"; + util::sort(util::range_n(std::begin(cstr), sizeof(cstr) - 1)); + EXPECT_EQ("dhowy"s, cstr); + } + { + // reverse sort by transform c to -c + // char cstr[] = "howdy"; + // util::sort_by(util::make_range(std::begin(cstr), null_terminated), + // [](char c) { return -c; }); + // EXPECT_EQ("ywohd"s, cstr); + } + { + // stable sort: move capitals to front, numbers to back + char mixed[] = "t5hH4E3erLL2e1O"; + util::stable_sort_by(util::make_range(std::begin(mixed), null_terminated), + [](char c) { return std::isupper(c)? 0: std::isdigit(c)? 2: 1; }); + EXPECT_EQ("HELLOthere54321"s, mixed); + } + { + // sort with user-provided less comparison function + std::vector X = {{0, 5}, {1, 4}, {2, 3}, {3, 2}, {4, 1}, {5, 0}}; + util::sort(X, [](const foo& l, const foo& r) {return l.y{{5, 0}, {4, 1}, {3, 2}, {2, 3}, {1, 4}, {0, 5}})); + util::sort(X, [](const foo& l, const foo& r) {return l.x{{0, 5}, {1, 4}, {2, 3}, {3, 2}, {4, 1}, {5, 0}})); + } } TEST(range, sum) { From 28a0cce695dd6caf31f1a884299804d7821f633a Mon Sep 17 00:00:00 2001 From: Thorsten Hater <24411438+thorstenhater@users.noreply.github.com> Date: Tue, 10 Sep 2024 09:08:40 +0200 Subject: [PATCH 5/7] Be extorted by GCC's weird views. --- test/unit/test_range.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/unit/test_range.cpp b/test/unit/test_range.cpp index e096a2e4a1..de86ef7ecd 100644 --- a/test/unit/test_range.cpp +++ b/test/unit/test_range.cpp @@ -444,20 +444,20 @@ TEST(range, sort) { { // simple sort char cstr[] = "howdy"; - util::sort(util::range_n(std::begin(cstr), sizeof(cstr) - 1)); + util::sort(util::range_n(cstr, 5)); EXPECT_EQ("dhowy"s, cstr); } { // reverse sort by transform c to -c - // char cstr[] = "howdy"; - // util::sort_by(util::make_range(std::begin(cstr), null_terminated), - // [](char c) { return -c; }); - // EXPECT_EQ("ywohd"s, cstr); + char cstr[] = "howdy"; + util::sort_by(util::range_n(cstr, 5), + [](char c) { return -c; }); + EXPECT_EQ("ywohd"s, cstr); } { // stable sort: move capitals to front, numbers to back char mixed[] = "t5hH4E3erLL2e1O"; - util::stable_sort_by(util::make_range(std::begin(mixed), null_terminated), + util::stable_sort_by(util::strict_view(util::make_range(std::begin(mixed), null_terminated)), [](char c) { return std::isupper(c)? 0: std::isdigit(c)? 2: 1; }); EXPECT_EQ("HELLOthere54321"s, mixed); } From eb4c56e748cf874a56ae0aebf382dcd1051c5119 Mon Sep 17 00:00:00 2001 From: Thorsten Hater <24411438+thorstenhater@users.noreply.github.com> Date: Fri, 20 Sep 2024 08:20:47 +0200 Subject: [PATCH 6/7] Honour ADL. --- arbor/util/range.hpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/arbor/util/range.hpp b/arbor/util/range.hpp index 5f01b6f4dd..5d26c3561a 100644 --- a/arbor/util/range.hpp +++ b/arbor/util/range.hpp @@ -156,8 +156,9 @@ range range_n(T t, size_t n) { template auto canonical_view(Seq&& s) { - auto b = std::begin(s); - auto e = std::end(s); + using std::begin; + auto b = begin(s); + auto e = end(s); return make_range(make_sentinel_iterator(b, e), make_sentinel_end(b, e)); } @@ -166,8 +167,9 @@ auto canonical_view(Seq&& s) { template auto strict_view(Seq&& s) { - auto b = std::begin(s); - auto e = std::end(s); + using std::begin; + auto b = begin(s); + auto e = end(s); return make_range(b, b==e? b: std::next(util::upto(b, e))); } From ddb60026c8717f05e7929fa1171ce288349501a2 Mon Sep 17 00:00:00 2001 From: Thorsten Hater <24411438+thorstenhater@users.noreply.github.com> Date: Fri, 18 Oct 2024 09:35:30 +0200 Subject: [PATCH 7/7] Update arbor/util/range.hpp Co-authored-by: boeschf <48126478+boeschf@users.noreply.github.com> --- arbor/util/range.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/arbor/util/range.hpp b/arbor/util/range.hpp index 5d26c3561a..13f80b095e 100644 --- a/arbor/util/range.hpp +++ b/arbor/util/range.hpp @@ -157,6 +157,7 @@ range range_n(T t, size_t n) { template auto canonical_view(Seq&& s) { using std::begin; + using std::end; auto b = begin(s); auto e = end(s); return make_range(make_sentinel_iterator(b, e), make_sentinel_end(b, e));