From 75d61277f542dc1875140cfaa1c2eba0bdb87479 Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Wed, 11 Jun 2025 10:04:37 +0200 Subject: [PATCH 01/18] remove duplicate test --- tests/test_selection.cpp | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/tests/test_selection.cpp b/tests/test_selection.cpp index 5504e16..1f1dbbe 100644 --- a/tests/test_selection.cpp +++ b/tests/test_selection.cpp @@ -102,32 +102,6 @@ TEST_CASE("Selection", "[base]") { CHECK(Selection({{0, 10}}) == (even | odd)); } - SECTION("union") { - const auto empty = Selection({}); - CHECK(empty == (empty | empty)); - - // clang-format off - // 1 2 - // 01234567890123456789012345 - // a = xx xxxxx xxxxxxxxxx x - // b = xxxxx xxxxx xxxxxxxx x - // xxxxxxxxxxxxxxxxxxxxxxx x - // clang-format on - const auto a = Selection({{24, 25}, {13, 23}, {5, 10}, {0, 2}}); - const auto b = Selection({{1, 6}, {8, 13}, {15, 23}, {24, 25}}); - CHECK(b == (b | empty)); // need to use b since it's sorted - CHECK(b == (empty | b)); - - const auto expected = Selection({{0, 23}, {24, 25}}); - CHECK(expected == (a | b)); - CHECK(expected == (b | a)); - - const auto odd = Selection::fromValues({1, 3, 5, 7, 9}); - const auto even = Selection::fromValues({0, 2, 4, 6, 8}); - CHECK(Selection({{0, 10}}) == (odd | even)); - CHECK(Selection({{0, 10}}) == (even | odd)); - } - SECTION("contains") { const auto sel = Selection({{2, 5}, {20, 21}, {10, 15}}); // unsorted ranges From eb80160dc27530ce2441defbaaf10026856b734e Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Wed, 11 Jun 2025 10:04:54 +0200 Subject: [PATCH 02/18] run clang-format on compartment_sets.{h,cpp} --- include/bbp/sonata/compartment_sets.h | 56 +++++------ src/compartment_sets.cpp | 133 +++++++++++++++----------- 2 files changed, 103 insertions(+), 86 deletions(-) diff --git a/include/bbp/sonata/compartment_sets.h b/include/bbp/sonata/compartment_sets.h index 802aab5..b2edd62 100644 --- a/include/bbp/sonata/compartment_sets.h +++ b/include/bbp/sonata/compartment_sets.h @@ -54,31 +54,32 @@ inline std::ostream& operator<<(std::ostream& os, const CompartmentLocation& cl) return os; } -class SONATA_API CompartmentSetFilteredIterator { -public: - using iterator_category = std::forward_iterator_tag; - using value_type = CompartmentLocation; - using difference_type = std::ptrdiff_t; - using pointer = const CompartmentLocation*; - using reference = const CompartmentLocation&; - - explicit CompartmentSetFilteredIterator( - std::unique_ptr impl); - CompartmentSetFilteredIterator(const CompartmentSetFilteredIterator& other); - CompartmentSetFilteredIterator& operator=(const CompartmentSetFilteredIterator& other); - CompartmentSetFilteredIterator(CompartmentSetFilteredIterator&&) noexcept; - CompartmentSetFilteredIterator& operator=(CompartmentSetFilteredIterator&&) noexcept; - ~CompartmentSetFilteredIterator(); - - const CompartmentLocation& operator*() const; - const CompartmentLocation* operator->() const; - - CompartmentSetFilteredIterator& operator++(); // prefix ++ - CompartmentSetFilteredIterator operator++(int); // postfix ++ - bool operator==(const CompartmentSetFilteredIterator& other) const; - bool operator!=(const CompartmentSetFilteredIterator& other) const; - -private: +class SONATA_API CompartmentSetFilteredIterator +{ + public: + using iterator_category = std::forward_iterator_tag; + using value_type = CompartmentLocation; + using difference_type = std::ptrdiff_t; + using pointer = const CompartmentLocation*; + using reference = const CompartmentLocation&; + + explicit CompartmentSetFilteredIterator( + std::unique_ptr impl); + CompartmentSetFilteredIterator(const CompartmentSetFilteredIterator& other); + CompartmentSetFilteredIterator& operator=(const CompartmentSetFilteredIterator& other); + CompartmentSetFilteredIterator(CompartmentSetFilteredIterator&&) noexcept; + CompartmentSetFilteredIterator& operator=(CompartmentSetFilteredIterator&&) noexcept; + ~CompartmentSetFilteredIterator(); + + const CompartmentLocation& operator*() const; + const CompartmentLocation* operator->() const; + + CompartmentSetFilteredIterator& operator++(); // prefix ++ + CompartmentSetFilteredIterator operator++(int); // postfix ++ + bool operator==(const CompartmentSetFilteredIterator& other) const; + bool operator!=(const CompartmentSetFilteredIterator& other) const; + + private: std::unique_ptr impl_; }; @@ -140,8 +141,7 @@ class SONATA_API CompartmentSet */ class SONATA_API CompartmentSets { -public: - + public: CompartmentSets(const std::string& content); CompartmentSets(std::unique_ptr&& impl); CompartmentSets(detail::CompartmentSets&& impl); @@ -181,7 +181,7 @@ class SONATA_API CompartmentSets bool operator==(const CompartmentSets& other) const; bool operator!=(const CompartmentSets& other) const; -private: + private: std::unique_ptr impl_; }; diff --git a/src/compartment_sets.cpp b/src/compartment_sets.cpp index 6f2f283..89d173d 100644 --- a/src/compartment_sets.cpp +++ b/src/compartment_sets.cpp @@ -17,8 +17,9 @@ namespace detail { using json = nlohmann::json; -class CompartmentSetFilteredIterator { -public: +class CompartmentSetFilteredIterator +{ + public: using base_iterator = std::vector::const_iterator; using value_type = CompartmentLocation; using reference = const value_type&; @@ -29,7 +30,7 @@ class CompartmentSetFilteredIterator { private: base_iterator current_; base_iterator end_; - bbp::sonata::Selection selection_; // copied + bbp::sonata::Selection selection_; // copied void skip_to_valid() { while (current_ != end_) { @@ -40,12 +41,13 @@ class CompartmentSetFilteredIterator { } } -public: - + public: CompartmentSetFilteredIterator(base_iterator current, - base_iterator end, - bbp::sonata::Selection selection) - : current_(current), end_(end), selection_(std::move(selection)) { + base_iterator end, + bbp::sonata::Selection selection) + : current_(current) + , end_(end) + , selection_(std::move(selection)) { skip_to_valid(); } CompartmentSetFilteredIterator(base_iterator end) @@ -86,22 +88,26 @@ class CompartmentSetFilteredIterator { } }; -class CompartmentSet { -public: +class CompartmentSet +{ + public: using container_t = std::vector; class FilteredIterator; -private: + + private: // Private constructor for filter factory method std::string population_; container_t compartment_locations_; - + /** * Copy-construction is private. Used only for cloning. */ CompartmentSet(const CompartmentSet& other) = default; - CompartmentSet(const std::string& population, const container_t& compartment_locations): population_(population), compartment_locations_(compartment_locations) {} + CompartmentSet(const std::string& population, const container_t& compartment_locations) + : population_(population) + , compartment_locations_(compartment_locations) { } static CompartmentLocation _parseCompartmentLocation(const nlohmann::json& j) { if (!j.is_array() || j.size() != 3) { @@ -125,11 +131,10 @@ class CompartmentSet { return {node_id, section_index, offset}; } -public: - + public: // Construct from JSON string (delegates to JSON constructor) explicit CompartmentSet(const std::string& content) - : CompartmentSet(nlohmann::json::parse(content)) {} + : CompartmentSet(nlohmann::json::parse(content)) { } // Construct from JSON object explicit CompartmentSet(const nlohmann::json& j) { @@ -160,11 +165,11 @@ class CompartmentSet { CompartmentSet(CompartmentSet&&) noexcept = default; CompartmentSet& operator=(CompartmentSet&&) noexcept = default; - std::pair - filtered_crange(bbp::sonata::Selection selection = Selection({})) const { + std::pair filtered_crange( + bbp::sonata::Selection selection = Selection({})) const { CompartmentSetFilteredIterator begin_it(compartment_locations_.cbegin(), - compartment_locations_.cend(), - selection); + compartment_locations_.cend(), + selection); CompartmentSetFilteredIterator end_it(compartment_locations_.cend()); return {begin_it, end_it}; } @@ -197,7 +202,7 @@ class CompartmentSet { result.reserve(compartment_locations_.size()); for (const auto& elem : compartment_locations_) { uint64_t id = elem.nodeId; - if (seen.insert(id).second) { // insert returns {iterator, bool} + if (seen.insert(id).second) { // insert returns {iterator, bool} result.push_back(id); } } @@ -226,7 +231,8 @@ class CompartmentSet { return std::unique_ptr(new CompartmentSet(*this)); } - std::unique_ptr filter(const bbp::sonata::Selection& selection = bbp::sonata::Selection({})) const { + std::unique_ptr filter( + const bbp::sonata::Selection& selection = bbp::sonata::Selection({})) const { if (selection.empty()) { return clone(); } @@ -237,12 +243,13 @@ class CompartmentSet { filtered.emplace_back(el); } } - return std::unique_ptr(new CompartmentSet(population_, std::move(filtered))); + return std::unique_ptr( + new CompartmentSet(population_, std::move(filtered))); } bool operator==(const CompartmentSet& other) const { return (population_ == other.population_) && - (compartment_locations_ == other.compartment_locations_); + (compartment_locations_ == other.compartment_locations_); } bool operator!=(const CompartmentSet& other) const { @@ -251,10 +258,10 @@ class CompartmentSet { }; class CompartmentSets { -private: + private: std::map> data_; -public: + public: CompartmentSets(const json& j) { if (!j.is_object()) { throw SonataError("Top level compartment_set must be an object"); @@ -273,7 +280,7 @@ class CompartmentSets } CompartmentSets(const fs::path& path) - : CompartmentSets(json::parse(std::ifstream(validate_path(path)))) {} + : CompartmentSets(json::parse(std::ifstream(validate_path(path)))) { } static CompartmentSets fromFile(const std::string& path_) { fs::path path(path_); @@ -281,7 +288,7 @@ class CompartmentSets } CompartmentSets(const std::string& content) - : CompartmentSets(json::parse(content)) {} + : CompartmentSets(json::parse(content)) { } std::shared_ptr getCompartmentSet(const std::string& key) const { @@ -303,16 +310,18 @@ class CompartmentSets std::vector names() const { std::vector result; result.reserve(data_.size()); - std::transform(data_.begin(), data_.end(), std::back_inserter(result), - [](const auto& kv) { return kv.first; }); + std::transform(data_.begin(), data_.end(), std::back_inserter(result), [](const auto& kv) { + return kv.first; + }); return result; } std::vector> getAllCompartmentSets() const { std::vector> result; result.reserve(data_.size()); - std::transform(data_.begin(), data_.end(), std::back_inserter(result), - [](const auto& kv) { return kv.second; }); + std::transform(data_.begin(), data_.end(), std::back_inserter(result), [](const auto& kv) { + return kv.second; + }); return result; } @@ -370,15 +379,18 @@ class CompartmentSets // CompartmentSetFilteredIterator public API // Constructor -CompartmentSetFilteredIterator::CompartmentSetFilteredIterator(std::unique_ptr impl) - : impl_(std::move(impl)) {} +CompartmentSetFilteredIterator::CompartmentSetFilteredIterator( + std::unique_ptr impl) + : impl_(std::move(impl)) { } // Copy constructor -CompartmentSetFilteredIterator::CompartmentSetFilteredIterator(const CompartmentSetFilteredIterator& other) - : impl_(other.impl_ ? other.impl_->clone() : nullptr) {} +CompartmentSetFilteredIterator::CompartmentSetFilteredIterator( + const CompartmentSetFilteredIterator& other) + : impl_(other.impl_ ? other.impl_->clone() : nullptr) { } // Copy assignment operator -CompartmentSetFilteredIterator& CompartmentSetFilteredIterator::operator=(const CompartmentSetFilteredIterator& other) { +CompartmentSetFilteredIterator& CompartmentSetFilteredIterator::operator=( + const CompartmentSetFilteredIterator& other) { if (this != &other) { impl_ = other.impl_ ? other.impl_->clone() : nullptr; } @@ -386,10 +398,12 @@ CompartmentSetFilteredIterator& CompartmentSetFilteredIterator::operator=(const } // Move constructor -CompartmentSetFilteredIterator::CompartmentSetFilteredIterator(CompartmentSetFilteredIterator&&) noexcept = default; +CompartmentSetFilteredIterator::CompartmentSetFilteredIterator( + CompartmentSetFilteredIterator&&) noexcept = default; // Move assignment operator -CompartmentSetFilteredIterator& CompartmentSetFilteredIterator::operator=(CompartmentSetFilteredIterator&&) noexcept = default; +CompartmentSetFilteredIterator& CompartmentSetFilteredIterator::operator=( + CompartmentSetFilteredIterator&&) noexcept = default; CompartmentSetFilteredIterator::~CompartmentSetFilteredIterator() = default; @@ -408,7 +422,8 @@ CompartmentSetFilteredIterator& CompartmentSetFilteredIterator::operator++() { } CompartmentSetFilteredIterator CompartmentSetFilteredIterator::operator++(int) { - CompartmentSetFilteredIterator tmp(std::make_unique(*impl_)); + CompartmentSetFilteredIterator tmp( + std::make_unique(*impl_)); ++(*impl_); return tmp; } @@ -424,10 +439,10 @@ bool CompartmentSetFilteredIterator::operator!=(const CompartmentSetFilteredIter // CompartmentSet public API CompartmentSet::CompartmentSet(const std::string& json_content) - : impl_(std::make_shared(json_content)) {} + : impl_(std::make_shared(json_content)) { } CompartmentSet::CompartmentSet(std::shared_ptr&& impl) - : impl_(std::move(impl)) {} + : impl_(std::move(impl)) { } std::pair @@ -435,10 +450,8 @@ CompartmentSet::filtered_crange(bbp::sonata::Selection selection) const { const auto internal_result = impl_->filtered_crange(std::move(selection)); // Wrap clones of detail iterators in public API iterators - return { - CompartmentSetFilteredIterator(internal_result.first.clone()), - CompartmentSetFilteredIterator(internal_result.second.clone()) - }; + return {CompartmentSetFilteredIterator(internal_result.first.clone()), + CompartmentSetFilteredIterator(internal_result.second.clone())}; } std::size_t CompartmentSet::size(const bbp::sonata::Selection& selection) const { @@ -480,18 +493,17 @@ std::string CompartmentSet::toJSON() const { // CompartmentSets public API CompartmentSets::CompartmentSets(const std::string& content) - : impl_(new detail::CompartmentSets(content)) {} + : impl_(new detail::CompartmentSets(content)) { } CompartmentSets::CompartmentSets(std::unique_ptr&& impl) - : impl_(std::move(impl)) {} + : impl_(std::move(impl)) { } CompartmentSets::CompartmentSets(detail::CompartmentSets&& impl) - : CompartmentSets(std::make_unique(impl)) {} + : CompartmentSets(std::make_unique(impl)) { } CompartmentSets::CompartmentSets(CompartmentSets&&) noexcept = default; CompartmentSets& CompartmentSets::operator=(CompartmentSets&&) noexcept = default; CompartmentSets::~CompartmentSets() = default; - CompartmentSets CompartmentSets::fromFile(const std::string& path) { return detail::CompartmentSets::fromFile(path); } @@ -525,8 +537,12 @@ std::vector CompartmentSets::getAllCompartmentSets() const { const auto vals = impl_->getAllCompartmentSets(); std::vector result; result.reserve(vals.size()); - std::transform(vals.begin(), vals.end(), std::back_inserter(result), - [](std::shared_ptr ptr) { return CompartmentSet(std::move(ptr)); }); + std::transform(vals.begin(), + vals.end(), + std::back_inserter(result), + [](std::shared_ptr ptr) { + return CompartmentSet(std::move(ptr)); + }); return result; } @@ -538,12 +554,13 @@ std::vector> CompartmentSets::items() con std::vector> result; result.reserve(items_vec.size()); - std::transform( - items_vec.begin(), items_vec.end(), - std::back_inserter(result), - [](auto kv) { // pass by value to own the shared_ptr - return std::make_pair(std::move(kv.first), CompartmentSet(std::move(kv.second))); - }); + std::transform(items_vec.begin(), + items_vec.end(), + std::back_inserter(result), + [](auto kv) { // pass by value to own the shared_ptr + return std::make_pair(std::move(kv.first), + CompartmentSet(std::move(kv.second))); + }); return result; } From 1710f6c328d68217db143b6a6d35ca9754266086 Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Wed, 11 Jun 2025 10:14:49 +0200 Subject: [PATCH 03/18] allow reports with compartment_sets --- include/bbp/sonata/config.h | 5 ++++- python/bindings.cpp | 3 ++- python/tests/test_config.py | 2 +- src/config.cpp | 17 ++++++++++++----- tests/data/config/simulation_config.json | 9 +++++++++ tests/test_config.cpp | 3 ++- 6 files changed, 30 insertions(+), 9 deletions(-) diff --git a/include/bbp/sonata/config.h b/include/bbp/sonata/config.h index 25c49e3..aa2be41 100644 --- a/include/bbp/sonata/config.h +++ b/include/bbp/sonata/config.h @@ -382,10 +382,11 @@ class SONATA_API SimulationConfig */ struct Report { enum class Sections { invalid = -1, soma, axon, dend, apic, all }; - enum class Type { invalid = -1, compartment, lfp, summation, synapse }; + enum class Type { invalid = -1, compartment, lfp, summation, synapse, compartment_set }; enum class Scaling { invalid = -1, none, area }; enum class Compartments { invalid = -1, center, all }; + /// Node sets on which to report std::string cells; /// Sections on which to report. Default value: "soma" @@ -398,6 +399,8 @@ class SONATA_API SimulationConfig /// For compartment type, select compartments to report. /// Default value: "center"(for sections: soma), "all"(for other sections) Compartments compartments; + /// Name of the compartment set (from compartment_set.json) used for generating the report. + std::string compartment_set; /// The simulation variable to access. The variables available are model dependent. For /// summation type, it supports multiple variables by comma separated strings. E.g. “ina”, /// "AdEx.V_M, v", "i_membrane, IClamp". diff --git a/python/bindings.cpp b/python/bindings.cpp index 1bc00c1..73fe6d9 100644 --- a/python/bindings.cpp +++ b/python/bindings.cpp @@ -918,7 +918,8 @@ PYBIND11_MODULE(_libsonata, m) { .value("compartment", SimulationConfig::Report::Type::compartment) .value("lfp", SimulationConfig::Report::Type::lfp) .value("summation", SimulationConfig::Report::Type::summation) - .value("synapse", SimulationConfig::Report::Type::synapse); + .value("synapse", SimulationConfig::Report::Type::synapse) + .value("compartment_set", SimulationConfig::Report::Type::compartment_set); py::enum_(report, "Scaling") .value("none", SimulationConfig::Report::Scaling::none) diff --git a/python/tests/test_config.py b/python/tests/test_config.py index 5391620..3747ca7 100644 --- a/python/tests/test_config.py +++ b/python/tests/test_config.py @@ -446,7 +446,7 @@ def test_basic(self): self.assertEqual(modifications["no_SK_E2"].section_configure, "%s.gSK_E2bar_SK_E2 = 0") self.assertEqual(self.config.list_report_names, - { "axonal_comp_centers", "cell_imembrane", "compartment", "soma", "lfp" }) + { "axonal_comp_centers", "cell_imembrane", "compartment", "soma", "lfp", "compartment_set_v" }) Report = SimulationConfig.Report self.assertEqual(self.config.report('soma').cells, 'Column') diff --git a/src/config.cpp b/src/config.cpp index f27a8cf..6ca44d3 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -83,7 +83,8 @@ NLOHMANN_JSON_SERIALIZE_ENUM(SimulationConfig::Report::Type, {SimulationConfig::Report::Type::compartment, "compartment"}, {SimulationConfig::Report::Type::lfp, "lfp"}, {SimulationConfig::Report::Type::summation, "summation"}, - {SimulationConfig::Report::Type::synapse, "synapse"}}) + {SimulationConfig::Report::Type::synapse, "synapse"}, + {SimulationConfig::Report::Type::compartment_set, "compartment_set"}}) NLOHMANN_JSON_SERIALIZE_ENUM(SimulationConfig::Report::Scaling, {{SimulationConfig::Report::Scaling::invalid, nullptr}, {SimulationConfig::Report::Scaling::none, "none"}, @@ -1146,14 +1147,20 @@ class SimulationConfig::Parser const std::string debugStr = "report " + it.key(); parseOptional(valueIt, "cells", report.cells, parseNodeSet()); - parseOptional(valueIt, "sections", report.sections, {Report::Sections::soma}); parseMandatory(valueIt, "type", debugStr, report.type); - parseOptional(valueIt, "scaling", report.scaling, {Report::Scaling::area}); + if (report.type == Report::Type::compartment_set && valueIt.find("sections") != valueIt.end()) { + throw SonataError("Field 'sections' is not allowed for reports of type 'compartment_set'."); + } + parseOptional(valueIt, "sections", report.sections, {report.type == Report::Type::compartment_set ? Report::Sections::invalid : Report::Sections::soma}); + if (report.type == Report::Type::compartment_set && valueIt.find("compartments") != valueIt.end()) { + throw SonataError("Field 'compartments' is not allowed for reports of type 'compartment_set'."); + } parseOptional(valueIt, "compartments", report.compartments, - {report.sections == Report::Sections::soma ? Report::Compartments::center - : Report::Compartments::all}); + {report.type == Report::Type::compartment_set ? Report::Compartments::invalid : (report.sections == Report::Sections::soma ? Report::Compartments::center + : Report::Compartments::all)}); + parseOptional(valueIt, "scaling", report.scaling, {Report::Scaling::area}); parseMandatory(valueIt, "variable_name", debugStr, report.variableName); parseOptional(valueIt, "unit", report.unit, {"mV"}); parseMandatory(valueIt, "dt", debugStr, report.dt); diff --git a/tests/data/config/simulation_config.json b/tests/data/config/simulation_config.json index 594750a..e570d54 100644 --- a/tests/data/config/simulation_config.json +++ b/tests/data/config/simulation_config.json @@ -308,6 +308,15 @@ "start_time": 0, "end_time": 500, "enabled": true + }, + "compartment_set_v": { + "type": "compartment_set", + "compartment_set": "cs0", + "variable_name": "v", + "unit": "mV", + "dt": 0.1, + "start_time": 0.0, + "end_time": 100.0 } } } diff --git a/tests/test_config.cpp b/tests/test_config.cpp index d168a27..463459f 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -365,7 +365,8 @@ TEST_CASE("SimulationConfig") { "cell_imembrane", "compartment", "soma", - "lfp" + "lfp", + "compartment_set_v" }); CHECK(config.getReport("soma").cells == "Column"); From e148b493170b4b0befe26f6aa943fd95663fe117 Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Wed, 11 Jun 2025 10:15:16 +0200 Subject: [PATCH 04/18] format --- src/config.cpp | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index 6ca44d3..cc30ab0 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -1148,18 +1148,29 @@ class SimulationConfig::Parser parseOptional(valueIt, "cells", report.cells, parseNodeSet()); parseMandatory(valueIt, "type", debugStr, report.type); - if (report.type == Report::Type::compartment_set && valueIt.find("sections") != valueIt.end()) { - throw SonataError("Field 'sections' is not allowed for reports of type 'compartment_set'."); + if (report.type == Report::Type::compartment_set && + valueIt.find("sections") != valueIt.end()) { + throw SonataError( + "Field 'sections' is not allowed for reports of type 'compartment_set'."); } - parseOptional(valueIt, "sections", report.sections, {report.type == Report::Type::compartment_set ? Report::Sections::invalid : Report::Sections::soma}); - if (report.type == Report::Type::compartment_set && valueIt.find("compartments") != valueIt.end()) { - throw SonataError("Field 'compartments' is not allowed for reports of type 'compartment_set'."); + parseOptional(valueIt, + "sections", + report.sections, + {report.type == Report::Type::compartment_set ? Report::Sections::invalid + : Report::Sections::soma}); + if (report.type == Report::Type::compartment_set && + valueIt.find("compartments") != valueIt.end()) { + throw SonataError( + "Field 'compartments' is not allowed for reports of type 'compartment_set'."); } parseOptional(valueIt, "compartments", report.compartments, - {report.type == Report::Type::compartment_set ? Report::Compartments::invalid : (report.sections == Report::Sections::soma ? Report::Compartments::center - : Report::Compartments::all)}); + {report.type == Report::Type::compartment_set + ? Report::Compartments::invalid + : (report.sections == Report::Sections::soma + ? Report::Compartments::center + : Report::Compartments::all)}); parseOptional(valueIt, "scaling", report.scaling, {Report::Scaling::area}); parseMandatory(valueIt, "variable_name", debugStr, report.variableName); parseOptional(valueIt, "unit", report.unit, {"mV"}); From b9815aad4c919ac150ed0c79e3c6f1841b2ae913 Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Wed, 11 Jun 2025 10:18:09 +0200 Subject: [PATCH 05/18] remove unnecessary/duplicate comments --- src/compartment_sets.cpp | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/src/compartment_sets.cpp b/src/compartment_sets.cpp index 89d173d..24f4faa 100644 --- a/src/compartment_sets.cpp +++ b/src/compartment_sets.cpp @@ -1,5 +1,3 @@ - - #include "../extlib/filesystem.hpp" #include "utils.h" // readFile @@ -101,9 +99,7 @@ class CompartmentSet container_t compartment_locations_; - /** - * Copy-construction is private. Used only for cloning. - */ + //Copy-construction is private. Used only for cloning. CompartmentSet(const CompartmentSet& other) = default; CompartmentSet(const std::string& population, const container_t& compartment_locations) : population_(population) @@ -376,19 +372,14 @@ class CompartmentSets } // namespace detail -// CompartmentSetFilteredIterator public API - -// Constructor CompartmentSetFilteredIterator::CompartmentSetFilteredIterator( std::unique_ptr impl) : impl_(std::move(impl)) { } -// Copy constructor CompartmentSetFilteredIterator::CompartmentSetFilteredIterator( const CompartmentSetFilteredIterator& other) : impl_(other.impl_ ? other.impl_->clone() : nullptr) { } -// Copy assignment operator CompartmentSetFilteredIterator& CompartmentSetFilteredIterator::operator=( const CompartmentSetFilteredIterator& other) { if (this != &other) { @@ -397,11 +388,9 @@ CompartmentSetFilteredIterator& CompartmentSetFilteredIterator::operator=( return *this; } -// Move constructor CompartmentSetFilteredIterator::CompartmentSetFilteredIterator( CompartmentSetFilteredIterator&&) noexcept = default; -// Move assignment operator CompartmentSetFilteredIterator& CompartmentSetFilteredIterator::operator=( CompartmentSetFilteredIterator&&) noexcept = default; @@ -436,8 +425,6 @@ bool CompartmentSetFilteredIterator::operator!=(const CompartmentSetFilteredIter return !(*this == other); } -// CompartmentSet public API - CompartmentSet::CompartmentSet(const std::string& json_content) : impl_(std::make_shared(json_content)) { } @@ -490,8 +477,6 @@ std::string CompartmentSet::toJSON() const { return impl_->to_json().dump(); } -// CompartmentSets public API - CompartmentSets::CompartmentSets(const std::string& content) : impl_(new detail::CompartmentSets(content)) { } CompartmentSets::CompartmentSets(std::unique_ptr&& impl) @@ -512,27 +497,22 @@ CompartmentSet CompartmentSets::getCompartmentSet(const std::string& key) const return CompartmentSet(impl_->getCompartmentSet(key)); } -// Number of compartment sets std::size_t CompartmentSets::size() const { return impl_->size(); } -// is empty? bool CompartmentSets::empty() const { return impl_->empty(); } -// Check if key exists bool CompartmentSets::contains(const std::string& key) const { return impl_->contains(key); } -// Get keys as set or vector (use vector here) std::vector CompartmentSets::names() const { return impl_->names(); } -// Get all compartment sets as vector std::vector CompartmentSets::getAllCompartmentSets() const { const auto vals = impl_->getAllCompartmentSets(); std::vector result; @@ -547,7 +527,6 @@ std::vector CompartmentSets::getAllCompartmentSets() const { return result; } -// Get items (key + compartment set) as vector of pairs std::vector> CompartmentSets::items() const { auto items_vec = impl_->items(); @@ -564,7 +543,7 @@ std::vector> CompartmentSets::items() con return result; } -// Serialize all compartment sets to JSON string + std::string CompartmentSets::toJSON() const { return impl_->to_json().dump(); } @@ -578,4 +557,4 @@ bool CompartmentSets::operator!=(const CompartmentSets& other) const { } } // namespace sonata -} // namespace bbp \ No newline at end of file +} // namespace bbp From 369631676c7e33ad8b83bd4c9acf20ab58c41f4b Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Wed, 11 Jun 2025 10:41:00 +0200 Subject: [PATCH 06/18] additional tests --- tests/test_config.cpp | 44 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 463459f..704c4f5 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -1025,6 +1025,48 @@ TEST_CASE("SimulationConfig") { })"; CHECK_THROWS_AS(SimulationConfig(contents, "./"), SonataError); } + { // The "compartments" key is not allowed in a report of type compartment_set + auto contents = R"({ + "run": { + "dt": 0.05, + "tstop": 1000, + "random_seed": 0 + }, + "reports": { + "test": { + "cells": "nodesetstring", + "compartments": "center", + "type": "compartment_set", + "variable_name": "variablestring", + "dt": 0.05, + "start_time": 0, + "end_time": 500 + } + } + })"; + CHECK_THROWS_AS(SimulationConfig(contents, "./"), SonataError); + } + { // The "sections" key is not allowed in a report of type compartment_set + auto contents = R"({ + "run": { + "dt": 0.05, + "tstop": 1000, + "random_seed": 0 + }, + "reports": { + "test": { + "cells": "nodesetstring", + "sections": "soma", + "type": "compartment_set", + "variable_name": "variablestring", + "dt": 0.05, + "start_time": 0, + "end_time": 500 + } + } + })"; + CHECK_THROWS_AS(SimulationConfig(contents, "./"), SonataError); + } { // Invalid compartments in a report object auto contents = R"({ "run": { @@ -1035,7 +1077,7 @@ TEST_CASE("SimulationConfig") { "test": { "cells": "nodesetstring", "compartments": "middle", - "type": "compartment", + "type": "compartment_set", "variable_name": "variablestring", "dt": 0.05, "start_time": 0, From 7f5ab4af1b83c8f2c420286ec3854570b86ef174 Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Wed, 11 Jun 2025 10:46:58 +0200 Subject: [PATCH 07/18] cleanup test_compartment_sets.py --- python/tests/test_compartment_sets.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/python/tests/test_compartment_sets.py b/python/tests/test_compartment_sets.py index d7e2ecf..1181577 100644 --- a/python/tests/test_compartment_sets.py +++ b/python/tests/test_compartment_sets.py @@ -11,6 +11,8 @@ PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), '../../tests/data') + + class TestCompartmentLocation(unittest.TestCase): def setUp(self): self.json = '''{ @@ -37,6 +39,8 @@ def test_repr_and_str(self): expected = "CompartmentLocation(4, 40, 0.9)" self.assertEqual(repr(loc), expected) self.assertEqual(str(loc), repr(loc)) # str should delegate to repr + + class TestCompartmentSet(unittest.TestCase): def setUp(self): self.json = '''{ @@ -53,7 +57,7 @@ def setUp(self): def test_population_property(self): self.assertIsInstance(self.cs.population, str) self.assertEqual(self.cs.population, "pop0") - + def test_size(self): self.assertEqual(self.cs.size(), 4) self.assertEqual(self.cs.size([1, 2]), 3) @@ -98,7 +102,7 @@ def test_toJSON_roundtrip(self): json_out = self.cs.toJSON() cs2 = CompartmentSet(json_out) self.assertEqual(cs2, self.cs) - + def test_equality(self): cs1 = CompartmentSet(self.json) cs2 = CompartmentSet(self.json) @@ -125,6 +129,7 @@ def test_repr_and_str(self): self.assertTrue(r.startswith("CompartmentSet(population")) self.assertEqual(s, r) + class TestCompartmentSets(unittest.TestCase): def setUp(self): # Load valid json string from file @@ -159,7 +164,7 @@ def test_keys_values_items(self): for k, v in items: self.assertIn(k, keys) self.assertIn(v, values) - + def test_equality(self): cs1 = CompartmentSets(self.json_str) cs2 = CompartmentSets(self.json_str) @@ -193,4 +198,4 @@ def test_repr_and_str(self): self.assertEqual(s, r) # str delegates to repr # repr should contain keys from the dict for key in self.cs.names(): - self.assertIn(str(key), r) \ No newline at end of file + self.assertIn(str(key), r) From edca4ebe05d8338207cbe0ade5a33cea1a026669 Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Wed, 11 Jun 2025 10:49:26 +0200 Subject: [PATCH 08/18] try different way to use git-clang-format to capture changes across full diff --- ci/check_clang_format.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/check_clang_format.sh b/ci/check_clang_format.sh index ae03f99..d97b146 100755 --- a/ci/check_clang_format.sh +++ b/ci/check_clang_format.sh @@ -19,7 +19,7 @@ set +u # ignore errors in virtualenv's activate source "$VENV/bin/activate" set -u -changes=$(git-clang-format 'HEAD~1' $DIRS_TO_FORMAT) +changes=$(git-clang-format --diff $(git merge-base HEAD master) HEAD $DIRS_TO_FORMAT) if [[ $(echo "$changes" | grep -n1 'changed files') ]]; then echo "The following files require changes to pass the current clang-format" echo "$changes" From 918ae3a874aff5ce97bbb4fffcde9b97453d0ffe Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Wed, 11 Jun 2025 12:03:37 +0200 Subject: [PATCH 09/18] add compartment_set in Reports --- python/bindings.cpp | 5 ++++ python/generated/docstrings.h | 2 ++ python/tests/test_config.py | 4 +++ src/config.cpp | 25 ++++++++++++------- tests/test_config.cpp | 47 +++++++++++++++++++++++++++++++++++ 5 files changed, 74 insertions(+), 9 deletions(-) diff --git a/python/bindings.cpp b/python/bindings.cpp index 73fe6d9..1c3ee04 100644 --- a/python/bindings.cpp +++ b/python/bindings.cpp @@ -869,6 +869,9 @@ PYBIND11_MODULE(_libsonata, m) { .def_readonly("cells", &SimulationConfig::Report::cells, DOC_SIMULATIONCONFIG(Report, cells)) + .def_readonly("compartment_set", + &SimulationConfig::Report::compartment_set, + DOC_SIMULATIONCONFIG(Report, compartmentSet)) .def_readonly("sections", &SimulationConfig::Report::sections, DOC_SIMULATIONCONFIG(Report, sections)) @@ -898,6 +901,7 @@ PYBIND11_MODULE(_libsonata, m) { DOC_SIMULATIONCONFIG(Report, enabled)); py::enum_(report, "Sections") + .value("invalid", SimulationConfig::Report::Sections::invalid) .value("soma", SimulationConfig::Report::Sections::soma, DOC_SIMULATIONCONFIG(Report, Sections, soma)) @@ -926,6 +930,7 @@ PYBIND11_MODULE(_libsonata, m) { .value("area", SimulationConfig::Report::Scaling::area); py::enum_(report, "Compartments") + .value("invalid", SimulationConfig::Report::Compartments::invalid) .value("center", SimulationConfig::Report::Compartments::center) .value("all", SimulationConfig::Report::Compartments::all); diff --git a/python/generated/docstrings.h b/python/generated/docstrings.h index b16bb2b..eaa4bf0 100644 --- a/python/generated/docstrings.h +++ b/python/generated/docstrings.h @@ -1156,6 +1156,8 @@ static const char *__doc_bbp_sonata_SimulationConfig_Report_Type_synapse = R"doc static const char *__doc_bbp_sonata_SimulationConfig_Report_cells = R"doc(Node sets on which to report)doc"; +static const char *__doc_bbp_sonata_SimulationConfig_Report_compartmentSet = R"doc(Compartment set on which to report)doc"; + static const char *__doc_bbp_sonata_SimulationConfig_Report_compartments = R"doc(For compartment type, select compartments to report. Default value: "center"(for sections: soma), "all"(for other sections))doc"; diff --git a/python/tests/test_config.py b/python/tests/test_config.py index 3747ca7..119b031 100644 --- a/python/tests/test_config.py +++ b/python/tests/test_config.py @@ -468,6 +468,10 @@ def test_basic(self): self.assertEqual(self.config.report('cell_imembrane').type.name, 'summation') self.assertEqual(self.config.report('cell_imembrane').variable_name, 'i_membrane, IClamp') self.assertEqual(self.config.report('lfp').type, Report.Type.lfp) + self.assertEqual(self.config.report('compartment_set_v').type, Report.Type.compartment_set) + self.assertEqual(self.config.report('compartment_set_v').sections, Report.Sections.invalid) + self.assertEqual(self.config.report('compartment_set_v').compartments, Report.Compartments.invalid) + self.assertEqual(self.config.report('compartment_set_v').compartment_set, "cs0") self.assertEqual(self.config.network, os.path.abspath(os.path.join(PATH, 'config/circuit_config.json'))) diff --git a/src/config.cpp b/src/config.cpp index cc30ab0..7466b25 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -1148,21 +1148,28 @@ class SimulationConfig::Parser parseOptional(valueIt, "cells", report.cells, parseNodeSet()); parseMandatory(valueIt, "type", debugStr, report.type); - if (report.type == Report::Type::compartment_set && - valueIt.find("sections") != valueIt.end()) { - throw SonataError( - "Field 'sections' is not allowed for reports of type 'compartment_set'."); + if (report.type == Report::Type::compartment_set){ + parseMandatory(valueIt, "compartment_set", debugStr, report.compartment_set); + if (valueIt.find("sections") != valueIt.end()) { + throw SonataError( + "Field 'sections' is not allowed for reports of type 'compartment_set'."); + } + if (report.type == Report::Type::compartment_set && + valueIt.find("compartments") != valueIt.end()) { + throw SonataError( + "Field 'compartments' is not allowed for reports of type 'compartment_set'."); + } + } else { + if (valueIt.find("compartment_set") != valueIt.end()) { + throw SonataError( + "Field 'compartment_set' is not allowed for reports of type 'compartment_set'."); + } } parseOptional(valueIt, "sections", report.sections, {report.type == Report::Type::compartment_set ? Report::Sections::invalid : Report::Sections::soma}); - if (report.type == Report::Type::compartment_set && - valueIt.find("compartments") != valueIt.end()) { - throw SonataError( - "Field 'compartments' is not allowed for reports of type 'compartment_set'."); - } parseOptional(valueIt, "compartments", report.compartments, diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 704c4f5..205023d 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -388,6 +388,10 @@ TEST_CASE("SimulationConfig") { CHECK(config.getReport("cell_imembrane").endTime == 500.); CHECK(config.getReport("cell_imembrane").variableName == "i_membrane, IClamp"); CHECK(config.getReport("lfp").type == SimulationConfig::Report::Type::lfp); + CHECK(config.getReport("compartment_set_v").type == SimulationConfig::Report::Type::compartment_set); + CHECK(config.getReport("compartment_set_v").sections == SimulationConfig::Report::Sections::invalid); + CHECK(config.getReport("compartment_set_v").compartments == SimulationConfig::Report::Compartments::invalid); + CHECK(config.getReport("compartment_set_v").compartment_set == "cs0"); CHECK_NOTHROW(nlohmann::json::parse(config.getExpandedJSON())); CHECK(config.getBasePath() == basePath.lexically_normal()); @@ -1034,6 +1038,7 @@ TEST_CASE("SimulationConfig") { }, "reports": { "test": { + "compartment_set": "cs0", "cells": "nodesetstring", "compartments": "center", "type": "compartment_set", @@ -1055,6 +1060,7 @@ TEST_CASE("SimulationConfig") { }, "reports": { "test": { + "compartment_set": "cs0", "cells": "nodesetstring", "sections": "soma", "type": "compartment_set", @@ -1067,6 +1073,47 @@ TEST_CASE("SimulationConfig") { })"; CHECK_THROWS_AS(SimulationConfig(contents, "./"), SonataError); } + { // The "compartment_set" key is necessary in a report of type compartment_set + auto contents = R"({ + "run": { + "dt": 0.05, + "tstop": 1000, + "random_seed": 0 + }, + "reports": { + "test": { + "cells": "nodesetstring", + "type": "compartment_set", + "variable_name": "variablestring", + "dt": 0.05, + "start_time": 0, + "end_time": 500 + } + } + })"; + CHECK_THROWS_AS(SimulationConfig(contents, "./"), SonataError); + } + { // The "compartment_set" key is not allowed when the report is not of type compartment_set + auto contents = R"({ + "run": { + "dt": 0.05, + "tstop": 1000, + "random_seed": 0 + }, + "reports": { + "test": { + "compartment_set": "cs0", + "cells": "nodesetstring", + "type": "compartment", + "variable_name": "variablestring", + "dt": 0.05, + "start_time": 0, + "end_time": 500 + } + } + })"; + CHECK_THROWS_AS(SimulationConfig(contents, "./"), SonataError); + } { // Invalid compartments in a report object auto contents = R"({ "run": { From 9424415fc3a382e23e846e9ab898bc99ce113d0f Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Wed, 11 Jun 2025 12:04:11 +0200 Subject: [PATCH 10/18] format --- src/config.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index 7466b25..a604460 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -1148,7 +1148,7 @@ class SimulationConfig::Parser parseOptional(valueIt, "cells", report.cells, parseNodeSet()); parseMandatory(valueIt, "type", debugStr, report.type); - if (report.type == Report::Type::compartment_set){ + if (report.type == Report::Type::compartment_set) { parseMandatory(valueIt, "compartment_set", debugStr, report.compartment_set); if (valueIt.find("sections") != valueIt.end()) { throw SonataError( @@ -1157,12 +1157,14 @@ class SimulationConfig::Parser if (report.type == Report::Type::compartment_set && valueIt.find("compartments") != valueIt.end()) { throw SonataError( - "Field 'compartments' is not allowed for reports of type 'compartment_set'."); + "Field 'compartments' is not allowed for reports of type " + "'compartment_set'."); } } else { if (valueIt.find("compartment_set") != valueIt.end()) { throw SonataError( - "Field 'compartment_set' is not allowed for reports of type 'compartment_set'."); + "Field 'compartment_set' is not allowed for reports of type " + "'compartment_set'."); } } parseOptional(valueIt, From cdc95819634f83c8c51c83c94655043987b849af Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Thu, 12 Jun 2025 12:29:34 +0200 Subject: [PATCH 11/18] typo for fromFile --- python/bindings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/bindings.cpp b/python/bindings.cpp index 1c3ee04..0b0c30e 100644 --- a/python/bindings.cpp +++ b/python/bindings.cpp @@ -632,7 +632,7 @@ PYBIND11_MODULE(_libsonata, m) { py::class_(m, "CompartmentSets") .def(py::init()) - .def_static("fromFile", &CompartmentSets::fromFile, py::arg("path")) + .def_static("from_file", &CompartmentSets::fromFile, py::arg("path")) .def("__contains__", &CompartmentSets::contains, py::arg("key"), From 0eff626540be2f79e40542f01da5e079ee6fb523 Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Thu, 12 Jun 2025 12:39:15 +0200 Subject: [PATCH 12/18] fix test --- python/tests/test_compartment_sets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/tests/test_compartment_sets.py b/python/tests/test_compartment_sets.py index d7e2ecf..0c0d5fc 100644 --- a/python/tests/test_compartment_sets.py +++ b/python/tests/test_compartment_sets.py @@ -183,7 +183,7 @@ def test_toJSON_roundtrip(self): self.assertEqual(self.cs, cs2) def test_static_fromFile(self): - cs_file = CompartmentSets.fromFile(os.path.join(PATH, 'compartment_sets.json')) + cs_file = CompartmentSets.from_file(os.path.join(PATH, 'compartment_sets.json')) self.assertEqual(cs_file, self.cs) def test_repr_and_str(self): From 7bae7032599e2c261011c9c0884e74d54cd5ae48 Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Thu, 12 Jun 2025 16:59:34 +0200 Subject: [PATCH 13/18] add gid sorting (preserving order) --- python/tests/test_compartment_sets.py | 18 +++++++++--------- src/compartment_sets.cpp | 7 +++++++ tests/test_compartment_sets.cpp | 21 ++++++++++++++++----- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/python/tests/test_compartment_sets.py b/python/tests/test_compartment_sets.py index 0c0d5fc..94dd034 100644 --- a/python/tests/test_compartment_sets.py +++ b/python/tests/test_compartment_sets.py @@ -24,17 +24,17 @@ def setUp(self): self.cs = CompartmentSet(self.json) def test_constructor_from_values(self): loc = self.cs[0] - self.assertEqual(loc.node_id, 4) - self.assertEqual(loc.section_index, 40) - self.assertAlmostEqual(loc.offset, 0.9) + self.assertEqual(loc.node_id, 3) + self.assertEqual(loc.section_index, 30) + self.assertAlmostEqual(loc.offset, 0.75) def test_equality(self): - self.assertEqual(self.cs[0], self.cs[1]) - self.assertNotEqual(self.cs[0], self.cs[2]) + self.assertEqual(self.cs[1], self.cs[2]) + self.assertNotEqual(self.cs[0], self.cs[1]) def test_repr_and_str(self): loc = self.cs[0] - expected = "CompartmentLocation(4, 40, 0.9)" + expected = "CompartmentLocation(3, 30, 0.75)" self.assertEqual(repr(loc), expected) self.assertEqual(str(loc), repr(loc)) # str should delegate to repr class TestCompartmentSet(unittest.TestCase): @@ -68,7 +68,7 @@ def test_getitem(self): def test_getitem_negative_index(self): loc = self.cs[-1] - self.assertEqual((loc.node_id, loc.section_index, loc.offset), (2, 20, 0.25)) + self.assertEqual((loc.node_id, loc.section_index, loc.offset), (3, 30, 0.75)) def test_getitem_out_of_bounds_raises(self): with self.assertRaises(IndexError): @@ -78,9 +78,9 @@ def test_getitem_out_of_bounds_raises(self): def test_iterators(self): node_ids = [loc.node_id for loc in self.cs] - self.assertEqual(node_ids, [1, 2, 3, 2]) + self.assertEqual(node_ids, [1, 2, 2, 3]) node_ids = [loc.node_id for loc in self.cs.filtered_iter([2, 3])] - self.assertEqual(node_ids, [2, 3, 2]) + self.assertEqual(node_ids, [2, 2, 3]) conv_to_list = list(self.cs.filtered_iter([2, 3])) self.assertTrue(all(isinstance(i, CompartmentLocation) for i in conv_to_list)) diff --git a/src/compartment_sets.cpp b/src/compartment_sets.cpp index 6f2f283..4629b3f 100644 --- a/src/compartment_sets.cpp +++ b/src/compartment_sets.cpp @@ -153,6 +153,13 @@ class CompartmentSet { compartment_locations_.emplace_back(CompartmentSet::_parseCompartmentLocation(el)); } compartment_locations_.shrink_to_fit(); + // sort by gid. Preserve relative ordering + std::stable_sort( + compartment_locations_.begin(), compartment_locations_.end(), + [](const CompartmentLocation& a, const CompartmentLocation& b) { + return a.nodeId < b.nodeId; + } + ); } ~CompartmentSet() = default; diff --git a/tests/test_compartment_sets.cpp b/tests/test_compartment_sets.cpp index 7fc37e1..3f5d646 100644 --- a/tests/test_compartment_sets.cpp +++ b/tests/test_compartment_sets.cpp @@ -76,10 +76,21 @@ TEST_CASE("CompartmentSet public API") { // Access elements by index REQUIRE(cs[0] == CompartmentLocation{1, 10, 0.5}); REQUIRE(cs[1] == CompartmentLocation{2, 20, 0.25}); - REQUIRE(cs[2] == CompartmentLocation{3, 30, 0.75}); - REQUIRE(cs[3] == CompartmentLocation{2, 20, 0.25}); + REQUIRE(cs[2] == CompartmentLocation{2, 20, 0.25}); + REQUIRE(cs[3] == CompartmentLocation{3, 30, 0.75}); REQUIRE_THROWS_AS(cs[4], std::out_of_range); - REQUIRE(cs.toJSON() == json::parse(json_content).dump()); + + auto expected = json::parse(json_content); + + std::stable_sort( + expected["compartment_set"].begin(), + expected["compartment_set"].end(), + [](const json& a, const json& b) { + return a[0] < b[0]; + } + ); + + REQUIRE(cs.toJSON() == expected.dump()); } SECTION("JSON constructor throws on invalid input") { @@ -131,13 +142,13 @@ TEST_CASE("CompartmentSet public API") { } REQUIRE(nodeIds.size() == 4); - REQUIRE((nodeIds == std::vector{1, 2, 3, 2})); + REQUIRE((nodeIds == std::vector{1, 2, 2, 3})); nodeIds.clear(); for (auto it = cs.filtered_crange(Selection::fromValues({2, 3})).first; it != pp.second; ++it) { nodeIds.push_back((*it).nodeId); } REQUIRE(nodeIds.size() == 3); - REQUIRE((nodeIds == std::vector{2, 3, 2})); + REQUIRE((nodeIds == std::vector{2, 2, 3})); } SECTION("Filter returns subset") { From a07114e21a05127f29f4db45de4e431459f660b8 Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Thu, 12 Jun 2025 17:00:00 +0200 Subject: [PATCH 14/18] format --- src/compartment_sets.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/compartment_sets.cpp b/src/compartment_sets.cpp index 4629b3f..269f38a 100644 --- a/src/compartment_sets.cpp +++ b/src/compartment_sets.cpp @@ -154,12 +154,11 @@ class CompartmentSet { } compartment_locations_.shrink_to_fit(); // sort by gid. Preserve relative ordering - std::stable_sort( - compartment_locations_.begin(), compartment_locations_.end(), - [](const CompartmentLocation& a, const CompartmentLocation& b) { - return a.nodeId < b.nodeId; - } - ); + std::stable_sort(compartment_locations_.begin(), + compartment_locations_.end(), + [](const CompartmentLocation& a, const CompartmentLocation& b) { + return a.nodeId < b.nodeId; + }); } ~CompartmentSet() = default; From fe57b5cd5c7b907e74600520598434197249fb28 Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Tue, 17 Jun 2025 15:32:32 +0200 Subject: [PATCH 15/18] last fixes --- include/bbp/sonata/compartment_sets.h | 26 ++++++++++++++++++++++---- setup.py | 2 +- src/compartment_sets.cpp | 18 +++++++++++------- tests/data/compartment_sets.json | 2 +- tests/test_compartment_sets.cpp | 21 ++++++++++++--------- 5 files changed, 47 insertions(+), 22 deletions(-) diff --git a/include/bbp/sonata/compartment_sets.h b/include/bbp/sonata/compartment_sets.h index 802aab5..c378293 100644 --- a/include/bbp/sonata/compartment_sets.h +++ b/include/bbp/sonata/compartment_sets.h @@ -30,26 +30,44 @@ class CompartmentSets; struct CompartmentLocation { public: uint64_t nodeId = 0; - uint64_t sectionIndex = 0; + uint64_t sectionId = 0; double offset = 0.0; /// Comparator. Used to compare vectors in CompartmentSet. More idiomatic than defining a /// comaprator on the fly bool operator==(const CompartmentLocation& other) const { - return nodeId == other.nodeId && sectionIndex == other.sectionIndex && + return nodeId == other.nodeId && sectionId == other.sectionId && offset == other.offset; } bool operator!=(const CompartmentLocation& other) const { return !(*this == other); } + + bool operator<(const CompartmentLocation& other) const { + if (nodeId != other.nodeId) return nodeId < other.nodeId; + if (sectionId != other.sectionId) return sectionId < other.sectionId; + return offset < other.offset; + } + + bool operator>(const CompartmentLocation& other) const { + return other < *this; + } + + bool operator<=(const CompartmentLocation& other) const { + return !(other < *this); + } + + bool operator>=(const CompartmentLocation& other) const { + return !(*this < other); + } }; /// Ostream << operator used by catch2 when there are problems for example inline std::ostream& operator<<(std::ostream& os, const CompartmentLocation& cl) { os << "CompartmentLocation(" << "nodeId: " << cl.nodeId << ", " - << "sectionIndex: " << cl.sectionIndex << ", " + << "sectionId: " << cl.sectionId << ", " << "offset: " << cl.offset << ")"; return os; } @@ -186,4 +204,4 @@ class SONATA_API CompartmentSets }; } // namespace sonata -} // namespace bbp +} // namespace bbp \ No newline at end of file diff --git a/setup.py b/setup.py index f5c4903..318e3db 100644 --- a/setup.py +++ b/setup.py @@ -58,7 +58,7 @@ def build_extension(self, ext): cmake_args = [ "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=" + extdir, "-DSONATA_TESTS={}".format(os.environ.get("SONATA_TESTS", "OFF")), - "-DEXTLIB_FROM_SUBMODULES=ON", + "-DEXTLIB_FROM_SUBMODULES=OFF", "-DSONATA_PYTHON=ON", "-DSONATA_VERSION=" + self.distribution.get_version(), "-DCMAKE_BUILD_TYPE={}".format(build_type), diff --git a/src/compartment_sets.cpp b/src/compartment_sets.cpp index 269f38a..e62aec9 100644 --- a/src/compartment_sets.cpp +++ b/src/compartment_sets.cpp @@ -153,12 +153,16 @@ class CompartmentSet { compartment_locations_.emplace_back(CompartmentSet::_parseCompartmentLocation(el)); } compartment_locations_.shrink_to_fit(); - // sort by gid. Preserve relative ordering - std::stable_sort(compartment_locations_.begin(), - compartment_locations_.end(), - [](const CompartmentLocation& a, const CompartmentLocation& b) { - return a.nodeId < b.nodeId; - }); + // check sort + for (size_t i = 1; i < compartment_locations_.size(); ++i) { + const auto& prev = compartment_locations_[i - 1]; + const auto& curr = compartment_locations_[i]; + if (curr <= prev) { + throw SonataError(fmt::format( + "CompartmentSet 'compartment_set' must be strictly sorted. " + "Found CompartmentLocation({}, {}, {}) after CompartmentLocation({}, {}, {})", curr.nodeId, curr.sectionId, curr.offset, prev.nodeId, prev.sectionId, prev.offset)); + } + } } ~CompartmentSet() = default; @@ -222,7 +226,7 @@ class CompartmentSet { j["compartment_set"] = nlohmann::json::array(); for (const auto& elem : compartment_locations_) { j["compartment_set"].push_back( - nlohmann::json::array({elem.nodeId, elem.sectionIndex, elem.offset})); + nlohmann::json::array({elem.nodeId, elem.sectionId, elem.offset})); } return j; diff --git a/tests/data/compartment_sets.json b/tests/data/compartment_sets.json index f141cb8..a8c1f1c 100644 --- a/tests/data/compartment_sets.json +++ b/tests/data/compartment_sets.json @@ -4,7 +4,7 @@ "compartment_set": [ [0, 10, 0.1], [0, 10, 0.2], - [0, 10, 0.1], + [0, 10, 0.3], [2, 3, 0.1], [3, 6, 0.3] ] diff --git a/tests/test_compartment_sets.cpp b/tests/test_compartment_sets.cpp index 3f5d646..c3ebe9b 100644 --- a/tests/test_compartment_sets.cpp +++ b/tests/test_compartment_sets.cpp @@ -22,7 +22,7 @@ TEST_CASE("CompartmentLocation public API") { SECTION("Construct from valid nodeId, section_idx, offset") { const auto& loc = cs[0]; REQUIRE(loc.nodeId == 1); - REQUIRE(loc.sectionIndex == 10); + REQUIRE(loc.sectionId == 10); REQUIRE(loc.offset == Approx(0.5)); REQUIRE(cs[0] == CompartmentLocation{1, 10, 0.5}); } @@ -38,6 +38,9 @@ TEST_CASE("CompartmentLocation public API") { REQUIRE_THROWS_AS(CompartmentSet(R"({"population": "pop0", "compartment_set": [ [1, 2, -0.1] ]})"), SonataError); REQUIRE_THROWS_AS(CompartmentSet(R"({"population": "pop0", "compartment_set": [ [-1, 2, 0.1] ]})"), SonataError); REQUIRE_THROWS_AS(CompartmentSet(R"({"population": "pop0", "compartment_set": [ [1, -2, 0.1] ]})"), SonataError); + REQUIRE_THROWS_AS(CompartmentSet(R"({"population": "pop0", "compartment_set": [ [1, 0, 0.5], [0, 0, 0.5] ]})"), SonataError); + REQUIRE_THROWS_AS(CompartmentSet(R"({"population": "pop0", "compartment_set": [ [0, 1, 0.5], [0, 0, 0.5] ]})"), SonataError); + REQUIRE_THROWS_AS(CompartmentSet(R"({"population": "pop0", "compartment_set": [ [0, 0, 0.6], [0, 0, 0.5] ]})"), SonataError); } SECTION("Equality operators") { @@ -61,8 +64,8 @@ TEST_CASE("CompartmentSet public API") { "compartment_set": [ [1, 10, 0.5], [2, 20, 0.25], - [3, 30, 0.75], - [2, 20, 0.25] + [2, 20, 0.250001], + [3, 30, 0.75] ] } )"; @@ -76,7 +79,7 @@ TEST_CASE("CompartmentSet public API") { // Access elements by index REQUIRE(cs[0] == CompartmentLocation{1, 10, 0.5}); REQUIRE(cs[1] == CompartmentLocation{2, 20, 0.25}); - REQUIRE(cs[2] == CompartmentLocation{2, 20, 0.25}); + REQUIRE(cs[2] == CompartmentLocation{2, 20, 0.250001}); REQUIRE(cs[3] == CompartmentLocation{3, 30, 0.75}); REQUIRE_THROWS_AS(cs[4], std::out_of_range); @@ -230,7 +233,7 @@ TEST_CASE("CompartmentSets public API") { "compartment_set": [ [0, 10, 0.1], [0, 10, 0.2], - [0, 10, 0.1], + [0, 10, 0.3], [2, 3, 0.1], [3, 6, 0.3] ] @@ -246,7 +249,7 @@ TEST_CASE("CompartmentSets public API") { "compartment_set": [ [0, 10, 0.1], [0, 10, 0.2], - [0, 10, 0.1], + [0, 10, 0.3], [2, 3, 0.1], [3, 6, 0.3] ] @@ -287,7 +290,7 @@ TEST_CASE("CompartmentSets public API") { "compartment_set": [ [0, 10, 0.1], [0, 10, 0.2], - [0, 10, 0.1], + [0, 10, 0.3], [2, 3, 0.1], [3, 6, 0.3] ] @@ -310,9 +313,9 @@ TEST_CASE("CompartmentSets public API") { "cs1": { "population": "pop1", "compartment_set": [ - [0, 10, 0.8], + [0, 10, 0.15], [0, 10, 0.2], - [0, 10, 0.1], + [0, 10, 0.3], [2, 3, 0.1], [3, 6, 0.3] ] From 1b9c2d3df89ccdea838186676ca451bed9e3b695 Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Tue, 17 Jun 2025 15:47:07 +0200 Subject: [PATCH 16/18] format --- ci/python_test.sh | 2 ++ include/bbp/sonata/compartment_sets.h | 9 +++++---- src/compartment_sets.cpp | 20 +++++++++++++------- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/ci/python_test.sh b/ci/python_test.sh index 543da86..af598f3 100755 --- a/ci/python_test.sh +++ b/ci/python_test.sh @@ -16,6 +16,8 @@ set -u $BIN/pip -v install --upgrade pip setuptools wheel +export CMAKE_ARGS="-DEXTLIB_FROM_SUBMODULES=ON" + which python which pip pip debug diff --git a/include/bbp/sonata/compartment_sets.h b/include/bbp/sonata/compartment_sets.h index c378293..70bba85 100644 --- a/include/bbp/sonata/compartment_sets.h +++ b/include/bbp/sonata/compartment_sets.h @@ -36,8 +36,7 @@ struct CompartmentLocation { /// Comparator. Used to compare vectors in CompartmentSet. More idiomatic than defining a /// comaprator on the fly bool operator==(const CompartmentLocation& other) const { - return nodeId == other.nodeId && sectionId == other.sectionId && - offset == other.offset; + return nodeId == other.nodeId && sectionId == other.sectionId && offset == other.offset; } bool operator!=(const CompartmentLocation& other) const { @@ -45,8 +44,10 @@ struct CompartmentLocation { } bool operator<(const CompartmentLocation& other) const { - if (nodeId != other.nodeId) return nodeId < other.nodeId; - if (sectionId != other.sectionId) return sectionId < other.sectionId; + if (nodeId != other.nodeId) + return nodeId < other.nodeId; + if (sectionId != other.sectionId) + return sectionId < other.sectionId; return offset < other.offset; } diff --git a/src/compartment_sets.cpp b/src/compartment_sets.cpp index e62aec9..e8297e4 100644 --- a/src/compartment_sets.cpp +++ b/src/compartment_sets.cpp @@ -155,15 +155,21 @@ class CompartmentSet { compartment_locations_.shrink_to_fit(); // check sort for (size_t i = 1; i < compartment_locations_.size(); ++i) { - const auto& prev = compartment_locations_[i - 1]; - const auto& curr = compartment_locations_[i]; - if (curr <= prev) { - throw SonataError(fmt::format( - "CompartmentSet 'compartment_set' must be strictly sorted. " - "Found CompartmentLocation({}, {}, {}) after CompartmentLocation({}, {}, {})", curr.nodeId, curr.sectionId, curr.offset, prev.nodeId, prev.sectionId, prev.offset)); + const auto& prev = compartment_locations_[i - 1]; + const auto& curr = compartment_locations_[i]; + if (curr <= prev) { + throw SonataError(fmt::format( + "CompartmentSet 'compartment_set' must be strictly sorted. " + "Found CompartmentLocation({}, {}, {}) after CompartmentLocation({}, {}, {})", + curr.nodeId, + curr.sectionId, + curr.offset, + prev.nodeId, + prev.sectionId, + prev.offset)); + } } } - } ~CompartmentSet() = default; CompartmentSet& operator=(const CompartmentSet&) = delete; From ba78ce2357ac5e10df725c9280d26a9d0cc8ba20 Mon Sep 17 00:00:00 2001 From: Alessandro Cattabiani Date: Tue, 17 Jun 2025 16:11:43 +0200 Subject: [PATCH 17/18] fix building problems --- ci/python_test.sh | 2 - python/bindings.cpp | 12 ++++-- python/generated/docstrings.h | 2 +- python/tests/test_compartment_sets.py | 62 ++++++++++++++++++++------- setup.py | 2 +- 5 files changed, 56 insertions(+), 24 deletions(-) diff --git a/ci/python_test.sh b/ci/python_test.sh index af598f3..543da86 100755 --- a/ci/python_test.sh +++ b/ci/python_test.sh @@ -16,8 +16,6 @@ set -u $BIN/pip -v install --upgrade pip setuptools wheel -export CMAKE_ARGS="-DEXTLIB_FROM_SUBMODULES=ON" - which python which pip pip debug diff --git a/python/bindings.cpp b/python/bindings.cpp index 0b0c30e..c790538 100644 --- a/python/bindings.cpp +++ b/python/bindings.cpp @@ -551,19 +551,23 @@ PYBIND11_MODULE(_libsonata, m) { py::class_(m, "CompartmentLocation") .def("__eq__", &CompartmentLocation::operator==) .def("__ne__", &CompartmentLocation::operator!=) + .def("__lt__", &CompartmentLocation::operator<) + .def("__le__", &CompartmentLocation::operator<=) + .def("__gt__", &CompartmentLocation::operator>) + .def("__ge__", &CompartmentLocation::operator>=) .def("__repr__", [](const CompartmentLocation& self) { return fmt::format("CompartmentLocation({}, {}, {})", self.nodeId, - self.sectionIndex, + self.sectionId, self.offset); }) .def("__str__", [](const CompartmentLocation& self) { return py::str(py::repr(py::cast(self))); }) .def_readonly("node_id", &CompartmentLocation::nodeId, DOC_COMPARTMENTLOCATION(nodeId)) - .def_readonly("section_index", - &CompartmentLocation::sectionIndex, - DOC_COMPARTMENTLOCATION(sectionIndex)) + .def_readonly("section_id", + &CompartmentLocation::sectionId, + DOC_COMPARTMENTLOCATION(sectionId)) .def_readonly("offset", &CompartmentLocation::offset, DOC_COMPARTMENTLOCATION(offset)); py::class_(m, "CompartmentSet") diff --git a/python/generated/docstrings.h b/python/generated/docstrings.h index eaa4bf0..82d6029 100644 --- a/python/generated/docstrings.h +++ b/python/generated/docstrings.h @@ -383,7 +383,7 @@ static const char *__doc_bbp_sonata_NodeSets_toJSON = R"doc(Return the nodesets static const char *__doc_bbp_sonata_CompartmentLocation_nodeId = R"doc(Id of the node.)doc"; -static const char *__doc_bbp_sonata_CompartmentLocation_sectionIndex = R"doc(Absolute section index. Progressive index that uniquely identifies the section. There is a mapping between neuron section names (i.e. dend[10]) and this index.)doc"; +static const char *__doc_bbp_sonata_CompartmentLocation_sectionId = R"doc(Absolute section id. Progressive index that uniquely identifies the section. It is different from the relative section index. There is a mapping between neuron section names (i.e. dend[10]) and this index.)doc"; static const char *__doc_bbp_sonata_CompartmentLocation_offset = R"doc(Offset of the compartment along the section)doc"; diff --git a/python/tests/test_compartment_sets.py b/python/tests/test_compartment_sets.py index 94dd034..5dd6050 100644 --- a/python/tests/test_compartment_sets.py +++ b/python/tests/test_compartment_sets.py @@ -17,24 +17,54 @@ def setUp(self): "population": "pop0", "compartment_set": [ [4, 40, 0.9], - [4, 40, 0.9], - [3, 30, 0.75] + [4, 41, 0.9], + [5, 30, 0.75] ] }''' self.cs = CompartmentSet(self.json) def test_constructor_from_values(self): loc = self.cs[0] - self.assertEqual(loc.node_id, 3) - self.assertEqual(loc.section_index, 30) - self.assertAlmostEqual(loc.offset, 0.75) + self.assertEqual(loc.node_id, 4) + self.assertEqual(loc.section_id, 40) + self.assertAlmostEqual(loc.offset, 0.9) - def test_equality(self): - self.assertEqual(self.cs[1], self.cs[2]) - self.assertNotEqual(self.cs[0], self.cs[1]) + def test_comparison_operators(self): + cs2 = CompartmentSet(self.json) + + # For == and != use self.cs and cs2 + self.assertTrue(self.cs[0] == cs2[0]) + self.assertFalse(self.cs[0] != cs2[0]) + self.assertFalse(self.cs[0] == cs2[1]) + self.assertTrue(self.cs[0] != cs2[1]) + + # For other comparisons use self.cs only + loc1 = self.cs[0] + loc2 = self.cs[1] + loc3 = self.cs[0] + + # Less than + self.assertTrue(loc1 < loc2 or loc2 < loc1) + self.assertFalse(loc1 < loc3) + + # Less than or equal + self.assertTrue(loc1 <= loc3) + self.assertTrue(loc1 <= loc2 or loc2 <= loc1) + + # Greater than + self.assertTrue(loc2 > loc1 or loc1 > loc2) + self.assertFalse(loc1 > loc3) + + # Greater than or equal + self.assertTrue(loc1 >= loc3) + self.assertTrue(loc1 >= loc2 or loc2 >= loc1) + + # Consistency check for self.cs pairs + for a, b in [(loc1, loc2), (loc1, loc3)]: + self.assertTrue((a < b) + (a == b) + (a > b) == 1) def test_repr_and_str(self): loc = self.cs[0] - expected = "CompartmentLocation(3, 30, 0.75)" + expected = "CompartmentLocation(4, 40, 0.9)" self.assertEqual(repr(loc), expected) self.assertEqual(str(loc), repr(loc)) # str should delegate to repr class TestCompartmentSet(unittest.TestCase): @@ -44,8 +74,8 @@ def setUp(self): "compartment_set": [ [1, 10, 0.5], [2, 20, 0.25], - [3, 30, 0.75], - [2, 20, 0.25] + [2, 20, 0.26], + [4, 20, 0.25] ] }''' self.cs = CompartmentSet(self.json) @@ -64,11 +94,11 @@ def test_len_dunder(self): def test_getitem(self): loc = self.cs[0] - self.assertEqual((loc.node_id, loc.section_index, loc.offset), (1, 10, 0.5)) + self.assertEqual((loc.node_id, loc.section_id, loc.offset), (1, 10, 0.5)) def test_getitem_negative_index(self): loc = self.cs[-1] - self.assertEqual((loc.node_id, loc.section_index, loc.offset), (3, 30, 0.75)) + self.assertEqual((loc.node_id, loc.section_id, loc.offset), (4, 20, 0.25)) def test_getitem_out_of_bounds_raises(self): with self.assertRaises(IndexError): @@ -78,15 +108,15 @@ def test_getitem_out_of_bounds_raises(self): def test_iterators(self): node_ids = [loc.node_id for loc in self.cs] - self.assertEqual(node_ids, [1, 2, 2, 3]) + self.assertEqual(node_ids, [1, 2, 2, 4]) node_ids = [loc.node_id for loc in self.cs.filtered_iter([2, 3])] - self.assertEqual(node_ids, [2, 2, 3]) + self.assertEqual(node_ids, [2, 2]) conv_to_list = list(self.cs.filtered_iter([2, 3])) self.assertTrue(all(isinstance(i, CompartmentLocation) for i in conv_to_list)) def test_node_ids(self): node_ids = self.cs.node_ids() - self.assertEqual(node_ids, Selection([1, 2, 3])) + self.assertEqual(node_ids, Selection([1, 2, 4])) def test_filter_identity(self): filtered = self.cs.filter() diff --git a/setup.py b/setup.py index 318e3db..f5c4903 100644 --- a/setup.py +++ b/setup.py @@ -58,7 +58,7 @@ def build_extension(self, ext): cmake_args = [ "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=" + extdir, "-DSONATA_TESTS={}".format(os.environ.get("SONATA_TESTS", "OFF")), - "-DEXTLIB_FROM_SUBMODULES=OFF", + "-DEXTLIB_FROM_SUBMODULES=ON", "-DSONATA_PYTHON=ON", "-DSONATA_VERSION=" + self.distribution.get_version(), "-DCMAKE_BUILD_TYPE={}".format(build_type), From 98f0bd06ee96ad1eb6d38e86468a6ff0a19b7e21 Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Wed, 18 Jun 2025 11:15:03 +0200 Subject: [PATCH 18/18] try again --- .github/workflows/clang_format_check.yaml | 8 ++++++-- ci/check_clang_format.sh | 8 +++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/clang_format_check.yaml b/.github/workflows/clang_format_check.yaml index fc68c09..937fe76 100644 --- a/.github/workflows/clang_format_check.yaml +++ b/.github/workflows/clang_format_check.yaml @@ -12,8 +12,12 @@ jobs: - name: Fetch repository uses: actions/checkout@v4 with: - fetch-depth: 2 + fetch-depth: 0 + - name: Install packages run: sudo apt-get update && sudo apt-get install python3-venv + - name: check_clang_format - run: ci/check_clang_format.sh + run: | + git fetch origin master + ci/check_clang_format.sh diff --git a/ci/check_clang_format.sh b/ci/check_clang_format.sh index d97b146..ee2339d 100755 --- a/ci/check_clang_format.sh +++ b/ci/check_clang_format.sh @@ -19,7 +19,13 @@ set +u # ignore errors in virtualenv's activate source "$VENV/bin/activate" set -u -changes=$(git-clang-format --diff $(git merge-base HEAD master) HEAD $DIRS_TO_FORMAT) +git branch --all +git merge-base HEAD remotes/origin/master + +changes_from="$(git merge-base HEAD remotes/origin/master)" +echo "Changes from: $changes_from" +git-clang-format --diff $changes_from HEAD $DIRS_TO_FORMAT +changes=$(git-clang-format --diff $changes_from HEAD $DIRS_TO_FORMAT) if [[ $(echo "$changes" | grep -n1 'changed files') ]]; then echo "The following files require changes to pass the current clang-format" echo "$changes"