Fix and polish Raw Mechanism Support in ModCC and arbor-build-catalog…

…ue (#2292)

- Add `--raw` to `modcc`'s catalogue processing.
- Check `gpu == none` in `arbor-build-catalogue`
- Fix some cheap signed/unsigned warnings

mechanisms/BuildModules.cmake

@@ -81,6 +81,10 @@ function("make_catalogue_standalone")
     endif()
   endforeach()
 
+  foreach(mech ${MK_CAT_CXX})
+    set(mk_cat_modcc_flags -r ${mech} ${mk_cat_modcc_flags})
+  endforeach()
+
   add_custom_command(OUTPUT            ${catalogue_${MK_CAT_NAME}_source}
                      DEPENDS           ${catalogue_${MK_CAT_NAME}_mods}
                      WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}

modcc/modcc.cpp

@@ -72,6 +72,7 @@ auto key_by_value(const Map& map, const V& v) -> decltype(map.begin()->first) {
 struct Options {
     std::string outprefix;
     std::vector<std::string> modfiles;
+    std::vector<std::string> rawfiles;
     std::string modulename;
     std::string catalogue;
     bool verbose = false;
@@ -140,6 +141,7 @@ const char* usage_str =
         "-S|--simd-abi          [Override SIMD ABI in generated code. Use /n suffix to force SIMD width to be size n. Examples: 'avx2', 'native/4', ...]\n"
         "-V|--verbose           [Toggle verbose mode]\n"
         "-A|--analyse           [Toggle analysis mode]\n"
+        "-r|--raw               [Add raw (CXX) mechanisms]\n"
         "-T|--trace-codegen     [Leave trace marks in generated source]\n"
         "<filenames>            [Files to be compiled]\n";
 
@@ -172,6 +174,7 @@ int main(int argc, char **argv) {
                 { popt.simd,                                             "-S", "--simd-abi" },
                 { to::set(popt.trace_codegen), to::flag,                 "-T", "--trace-codegen"},
                 { to::action(add_target, to::keywords(targetKindMap)),   "-t", "--target" },
+                { to::push_back(opt.rawfiles),                           "-r", "--raw"},
                 { to::action(help), to::flag, to::exit,                  "-h", "--help" }
         };
 
@@ -290,24 +293,29 @@ int main(int argc, char **argv) {
     }
 
     if (!opt.catalogue.empty()) {
+        std::vector<std::string> names = {};
+        for (const auto& [mod, prefix]: modules) names.push_back(mod);
+        for (const auto& mod: opt.rawfiles) names.push_back(mod);
+
         const auto ns = std::regex_replace(popt.cpp_namespace, std::regex{"::"}, "_");
         {
-            std::ofstream out(outdir / (opt.catalogue + "_catalogue.cpp"));
-            out << "// Automatically generated by modcc\n"
+            auto fn = outdir / (opt.catalogue + "_catalogue.cpp");
+            std::ofstream out(fn);
+             out << "// Automatically generated by modcc\n"
                    "\n"
                    "#include <arbor/mechanism_abi.h>\n"
                    "\n";
 
-            for (const auto& [mod, prefix]: modules) out << fmt::format("#include \"{}.hpp\"\n", mod);
+            for (const auto& mod: names) out << fmt::format("#include \"{}.hpp\"\n", mod);
 
             out << "\n"
                    "#ifdef STANDALONE\n"
                    "extern \"C\" {\n"
                    "    [[gnu::visibility(\"default\")]] const void* get_catalogue(int* n) {\n"
                 << fmt::format("        *n = {0};\n"
                                "        static arb_mechanism cat[{0}] = {{\n",
-                               opt.modfiles.size());
-            for (const auto& [mod, prefix]: modules) {
+                               names.size());
+            for (const auto& mod: names) {
                 out << fmt::format("            make_{}_{}(),\n", ns, mod);
             }
             out << "        };\n"
@@ -326,7 +334,7 @@ int main(int argc, char **argv) {
                                "mechanism_catalogue build_{0}_catalogue() {{\n"
                                "    mechanism_catalogue cat;\n",
                                opt.catalogue);
-            for (const auto& [mod, prefix]: modules) {
+            for (const auto& mod: names) {
                 out << fmt::format("    {{\n"
                                    "        auto mech = make_{}_{}();\n"
                                    "        auto ty = mech.type();\n"

python/test/unit/test_io.py

@@ -224,7 +224,7 @@ def num_cells(self):
     def cell_kind(self, _):
         return A.cell_kind.cable
 
-    def cell_description(self, gid):
+    def cell_description(self, _):
         tree = A.segment_tree()
         s = tree.append(A.mnpos, A.mpoint(-3, 0, 0, 3), A.mpoint(3, 0, 0, 3), tag=1)
         _ = tree.append(s, A.mpoint(3, 0, 0, 1), A.mpoint(33, 0, 0, 1), tag=3)
@@ -235,7 +235,7 @@ def cell_description(self, gid):
 
         return A.cable_cell(tree, dec)
 
-    def global_properties(self, kind):
+    def global_properties(self, _):
         return self.the_props
 
 

scripts/arbor-build-catalogue

@@ -296,7 +296,7 @@ with TemporaryDirectory() as tmp:
     sfx = [".hpp"]
     if cpu:
         sfx += ["_cpu.cpp"]
-    if gpu:
+    if gpu and gpu != 'none':
         sfx += ["_gpu.cpp", "_gpu.cu"]
     for e in raw:
         for s in sfx:

test/unit-distributed/test_distributed_for_each.cpp

@@ -19,7 +19,7 @@ TEST(distributed_for_each, all_zero) {
     int call_count = 0;
 
     auto sample = [&](const util::range<int*>& range) {
-        EXPECT_EQ(0, range.size());
+        EXPECT_EQ(0ul, range.size());
         ++call_count;
     };
 

test/unit/test_asc.cpp

@@ -342,7 +342,7 @@ TEST(asc, spine) {
         EXPECT_EQ(m.branch_segments(6)[0].prox, (arb::mpoint{ 0, 5, 0, 1}));
         // Now check metadata
         auto d = std::get<arborio::asc_metadata>(result.metadata);
-        EXPECT_EQ(d.spines.size(), 2);
+        EXPECT_EQ(d.spines.size(), 2ul);
         EXPECT_EQ(d.spines[0].location.x, 11);
         EXPECT_EQ(d.spines[0].location.y, 12);
         EXPECT_EQ(d.spines[0].location.z, 13);
@@ -355,12 +355,12 @@ TEST(asc, spine) {
         EXPECT_EQ(d.spines[1].location.radius, 2);
         EXPECT_EQ(d.spines[1].name, "S2");
 
-        EXPECT_EQ(d.markers.size(), 2);
-        EXPECT_EQ(d.markers[0].locations.size(), 2);
+        EXPECT_EQ(d.markers.size(), 2ul);
+        EXPECT_EQ(d.markers[0].locations.size(), 2ul);
         EXPECT_EQ(d.markers[0].name, "M1");
         EXPECT_EQ(d.markers[0].marker, arborio::asc_marker::cross);
 
-        EXPECT_EQ(d.markers[1].locations.size(), 1);
+        EXPECT_EQ(d.markers[1].locations.size(), 1ul);
         EXPECT_EQ(d.markers[1].name, "M2");
         EXPECT_EQ(d.markers[1].marker, arborio::asc_marker::dot);
     }

test/unit/test_hash.cpp

@@ -20,5 +20,5 @@ TEST(hash, doesnt_compile) {
 
 // check that we do not fall into the trap of the STL...
 TEST(hash, integral_is_not_identity) {
-    ASSERT_NE(arb::hash_value(42), 42);
+    ASSERT_NE(arb::hash_value(42), 42ul);
 }