From 907f4a224df303dc5e7bd2822c63cd90be6d97c9 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 25 Nov 2021 17:44:00 +0100 Subject: [PATCH 01/16] Create getIds --- include/bbp/sonata/report_reader.h | 10 +++ src/report_reader.cpp | 110 ++++++++++++++++++----------- 2 files changed, 78 insertions(+), 42 deletions(-) diff --git a/include/bbp/sonata/report_reader.h b/include/bbp/sonata/report_reader.h index cf7ac457..a97737e5 100644 --- a/include/bbp/sonata/report_reader.h +++ b/include/bbp/sonata/report_reader.h @@ -122,8 +122,17 @@ class SONATA_API ReportReader * Return true if the data is sorted. */ bool getSorted() const; + + /** + * Return all the node ids. + */ std::vector getNodeIds() const; + /** + * Return selected ids. + */ + typename DataFrame::DataType getIds(const nonstd::optional& node_ids = nonstd::nullopt) const; + /** * \param node_ids limit the report to the given selection. * \param tstart return voltages occurring on or after tstart. tstart=nonstd::nullopt @@ -149,6 +158,7 @@ class SONATA_API ReportReader std::string time_units_; std::string data_units_; bool nodes_ids_sorted_ = false; + Selection::Values node_ids_from_selection(const nonstd::optional& node_ids = nonstd::nullopt) const; friend ReportReader; }; diff --git a/src/report_reader.cpp b/src/report_reader.cpp index 25e94bf7..946ea2c1 100644 --- a/src/report_reader.cpp +++ b/src/report_reader.cpp @@ -279,6 +279,30 @@ std::vector ReportReader::Population::getNodeIds() const { return nodes_ids_; } +template +Selection::Values ReportReader::Population::node_ids_from_selection(const nonstd::optional& selection) const { + // Simplify selection + // We should remove duplicates + // And when we can work with ranges let's sort them + // auto nodes_ids_ = Selection::fromValues(node_ids.flatten().sort()); + Selection::Values node_ids; + + if (!selection) { // Take all nodes in this case + node_ids.reserve(nodes_pointers_.size()); + std::transform(nodes_pointers_.begin(), + nodes_pointers_.end(), + std::back_inserter(node_ids), + [](const std::pair& node_pointer) { + return node_pointer.first; + }); + } else if (selection->empty()) { + return {}; + } else { + node_ids = selection->flatten(); + } + return node_ids; +} + template std::pair ReportReader::Population::getIndex( const nonstd::optional& tstart, const nonstd::optional& tstop) const { @@ -315,68 +339,55 @@ std::pair ReportReader::Population::getIndex( template -DataFrame ReportReader::Population::get(const nonstd::optional& selection, - const nonstd::optional& tstart, - const nonstd::optional& tstop, - const nonstd::optional& tstride) const { - DataFrame data_frame; - size_t index_start = 0; - size_t index_stop = 0; - std::tie(index_start, index_stop) = getIndex(tstart, tstop); - const size_t stride = tstride.value_or(1); - if (stride == 0) { - throw SonataError("tstride should be > 0"); - } - if (index_start > index_stop) { - throw SonataError("tstart should be <= to tstop"); - } +typename DataFrame::DataType ReportReader::Population::getIds(const nonstd::optional& selection) const { + typename DataFrame::DataType ids{}; - for (size_t i = index_start; i <= index_stop; i += stride) { - data_frame.times.push_back(times_index_[i].second); - } // Simplify selection // We should remove duplicates // And when we can work with ranges let's sort them // auto nodes_ids_ = Selection::fromValues(node_ids.flatten().sort()); - Selection::Values node_ids; + Selection::Values node_ids = node_ids_from_selection(selection); - if (!selection) { // Take all nodes in this case - node_ids.reserve(nodes_pointers_.size()); - std::transform(nodes_pointers_.begin(), - nodes_pointers_.end(), - std::back_inserter(node_ids), - [](const std::pair& node_pointer) { - return node_pointer.first; - }); - } else if (selection->empty()) { - return DataFrame{{}, {}, {}}; - } else { - node_ids = selection->flatten(); - } - - Ranges positions; - // min and max offsets of the node_ids requested are calculated - // to reduce the amount of IO that is brought to memory - uint64_t min = std::numeric_limits::max(); - uint64_t max = std::numeric_limits::min(); auto dataset_elem_ids = pop_group_.getGroup("mapping").getDataSet("element_ids"); for (const auto& node_id : node_ids) { const auto it = nodes_pointers_.find(node_id); if (it == nodes_pointers_.end()) { continue; } - min = std::min(it->second.first, min); - max = std::max(it->second.second, max); - positions.emplace_back(it->second.first, it->second.second); std::vector element_ids(it->second.second - it->second.first); dataset_elem_ids.select({it->second.first}, {it->second.second - it->second.first}) .read(element_ids.data()); for (const auto& elem : element_ids) { - data_frame.ids.push_back(make_key(node_id, elem)); + ids.push_back(make_key(node_id, elem)); } } + return ids; +} + +template +DataFrame ReportReader::Population::get(const nonstd::optional& selection, + const nonstd::optional& tstart, + const nonstd::optional& tstop, + const nonstd::optional& tstride) const { + DataFrame data_frame; + size_t index_start = 0; + size_t index_stop = 0; + std::tie(index_start, index_stop) = getIndex(tstart, tstop); + const size_t stride = tstride.value_or(1); + if (stride == 0) { + throw SonataError("tstride should be > 0"); + } + if (index_start > index_stop) { + throw SonataError("tstart should be <= to tstop"); + } + + for (size_t i = index_start; i <= index_stop; i += stride) { + data_frame.times.push_back(times_index_[i].second); + } + + data_frame.ids = getIds(selection); if (data_frame.ids.empty()) { // At the end no data available (wrong node_ids?) return DataFrame{{}, {}, {}}; } @@ -393,6 +404,21 @@ DataFrame ReportReader::Population::get(const nonstd::optional& fmt::format("DataType of dataset 'data' should be Float32 ('{}' was found)", dataset_type.string())); } + // min and max offsets of the node_ids requested are calculated + // to reduce the amount of IO that is brought to memory + uint64_t min = std::numeric_limits::max(); + uint64_t max = std::numeric_limits::min(); + const auto node_ids = node_ids_from_selection(selection); + Ranges positions; + for (const auto& node_id : node_ids) { + const auto it = nodes_pointers_.find(node_id); + if (it == nodes_pointers_.end()) { + continue; + } + min = std::min(it->second.first, min); + max = std::max(it->second.second, max); + positions.emplace_back(it->second.first, it->second.second); + } std::vector buffer(max - min); for (size_t timer_index = index_start; timer_index <= index_stop; timer_index += stride) { // Note: The code assumes that the file is chunked by rows and not by columns From 669e2e1b3aba9b238ac119b5b9ae4e925e969286 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 25 Nov 2021 18:02:50 +0100 Subject: [PATCH 02/16] clang-format --- include/bbp/sonata/report_reader.h | 6 ++++-- src/report_reader.cpp | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/include/bbp/sonata/report_reader.h b/include/bbp/sonata/report_reader.h index a97737e5..2bc32992 100644 --- a/include/bbp/sonata/report_reader.h +++ b/include/bbp/sonata/report_reader.h @@ -131,7 +131,8 @@ class SONATA_API ReportReader /** * Return selected ids. */ - typename DataFrame::DataType getIds(const nonstd::optional& node_ids = nonstd::nullopt) const; + typename DataFrame::DataType getIds( + const nonstd::optional& node_ids = nonstd::nullopt) const; /** * \param node_ids limit the report to the given selection. @@ -158,7 +159,8 @@ class SONATA_API ReportReader std::string time_units_; std::string data_units_; bool nodes_ids_sorted_ = false; - Selection::Values node_ids_from_selection(const nonstd::optional& node_ids = nonstd::nullopt) const; + Selection::Values node_ids_from_selection( + const nonstd::optional& node_ids = nonstd::nullopt) const; friend ReportReader; }; diff --git a/src/report_reader.cpp b/src/report_reader.cpp index 946ea2c1..be8b3f57 100644 --- a/src/report_reader.cpp +++ b/src/report_reader.cpp @@ -280,7 +280,8 @@ std::vector ReportReader::Population::getNodeIds() const { } template -Selection::Values ReportReader::Population::node_ids_from_selection(const nonstd::optional& selection) const { +Selection::Values ReportReader::Population::node_ids_from_selection( + const nonstd::optional& selection) const { // Simplify selection // We should remove duplicates // And when we can work with ranges let's sort them @@ -339,7 +340,8 @@ std::pair ReportReader::Population::getIndex( template -typename DataFrame::DataType ReportReader::Population::getIds(const nonstd::optional& selection) const { +typename DataFrame::DataType ReportReader::Population::getIds( + const nonstd::optional& selection) const { typename DataFrame::DataType ids{}; From fff985818a0fc189e4a72aed0e289e08c4af408c Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 25 Nov 2021 18:46:05 +0100 Subject: [PATCH 03/16] Fix doc --- python/generated/docstrings.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/python/generated/docstrings.h b/python/generated/docstrings.h index ac49a0b2..54666873 100644 --- a/python/generated/docstrings.h +++ b/python/generated/docstrings.h @@ -429,9 +429,11 @@ Parameter ``tstride``: static const char *__doc_bbp_sonata_ReportReader_Population_getDataUnits = R"doc(Return the unit of data.)doc"; +static const char *__doc_bbp_sonata_ReportReader_Population_getIds = R"doc(Return selected ids.)doc"; + static const char *__doc_bbp_sonata_ReportReader_Population_getIndex = R"doc()doc"; -static const char *__doc_bbp_sonata_ReportReader_Population_getNodeIds = R"doc()doc"; +static const char *__doc_bbp_sonata_ReportReader_Population_getNodeIds = R"doc(Return all the node ids.)doc"; static const char *__doc_bbp_sonata_ReportReader_Population_getSorted = R"doc(Return true if the data is sorted.)doc"; @@ -439,6 +441,8 @@ static const char *__doc_bbp_sonata_ReportReader_Population_getTimeUnits = R"doc static const char *__doc_bbp_sonata_ReportReader_Population_getTimes = R"doc(Return (tstart, tstop, tstep) of the population)doc"; +static const char *__doc_bbp_sonata_ReportReader_Population_node_ids_from_selection = R"doc()doc"; + static const char *__doc_bbp_sonata_ReportReader_Population_nodes_ids = R"doc()doc"; static const char *__doc_bbp_sonata_ReportReader_Population_nodes_ids_sorted = R"doc()doc"; From 804213fb13f6a38b6d633ab5b91486efd525e303 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Fri, 26 Nov 2021 15:03:32 +0100 Subject: [PATCH 04/16] add a callback for having only one loop --- CMakeLists.txt | 1 - include/bbp/sonata/report_reader.h | 3 ++- src/report_reader.cpp | 31 ++++++++++++++---------------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d70bfe9c..8ebf2852 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -56,7 +56,6 @@ else() endif() if (CMAKE_BUILD_TYPE STREQUAL "Debug") - set(ENABLE_COVERAGE ON) else() set(ENABLE_COVERAGE OFF) endif() diff --git a/include/bbp/sonata/report_reader.h b/include/bbp/sonata/report_reader.h index 2bc32992..f55f8253 100644 --- a/include/bbp/sonata/report_reader.h +++ b/include/bbp/sonata/report_reader.h @@ -132,7 +132,8 @@ class SONATA_API ReportReader * Return selected ids. */ typename DataFrame::DataType getIds( - const nonstd::optional& node_ids = nonstd::nullopt) const; + const nonstd::optional& node_ids = nonstd::nullopt, + std::function = nullptr) const; /** * \param node_ids limit the report to the given selection. diff --git a/src/report_reader.cpp b/src/report_reader.cpp index be8b3f57..78457f11 100644 --- a/src/report_reader.cpp +++ b/src/report_reader.cpp @@ -341,7 +341,7 @@ std::pair ReportReader::Population::getIndex( template typename DataFrame::DataType ReportReader::Population::getIds( - const nonstd::optional& selection) const { + const nonstd::optional& selection, std::function fun) const { typename DataFrame::DataType ids{}; @@ -364,6 +364,9 @@ typename DataFrame::DataType ReportReader::Population::getIds( for (const auto& elem : element_ids) { ids.push_back(make_key(node_id, elem)); } + + if (fun) + fun(it->second); } return ids; } @@ -389,7 +392,16 @@ DataFrame ReportReader::Population::get(const nonstd::optional& data_frame.times.push_back(times_index_[i].second); } - data_frame.ids = getIds(selection); + // min and max offsets of the node_ids requested are calculated + // to reduce the amount of IO that is brought to memory + Ranges positions; + uint64_t min = std::numeric_limits::max(); + uint64_t max = std::numeric_limits::min(); + data_frame.ids = getIds(selection, [&](const Range& range) { + min = std::min(range.first, min); + max = std::max(range.second, max); + positions.emplace_back(range.first, range.second); + }); if (data_frame.ids.empty()) { // At the end no data available (wrong node_ids?) return DataFrame{{}, {}, {}}; } @@ -406,21 +418,6 @@ DataFrame ReportReader::Population::get(const nonstd::optional& fmt::format("DataType of dataset 'data' should be Float32 ('{}' was found)", dataset_type.string())); } - // min and max offsets of the node_ids requested are calculated - // to reduce the amount of IO that is brought to memory - uint64_t min = std::numeric_limits::max(); - uint64_t max = std::numeric_limits::min(); - const auto node_ids = node_ids_from_selection(selection); - Ranges positions; - for (const auto& node_id : node_ids) { - const auto it = nodes_pointers_.find(node_id); - if (it == nodes_pointers_.end()) { - continue; - } - min = std::min(it->second.first, min); - max = std::max(it->second.second, max); - positions.emplace_back(it->second.first, it->second.second); - } std::vector buffer(max - min); for (size_t timer_index = index_start; timer_index <= index_stop; timer_index += stride) { // Note: The code assumes that the file is chunked by rows and not by columns From b65d85d8245bf53cfd1e1dd42d90bfe4bd396517 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Fri, 26 Nov 2021 15:08:45 +0100 Subject: [PATCH 05/16] oups --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8ebf2852..d70bfe9c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -56,6 +56,7 @@ else() endif() if (CMAKE_BUILD_TYPE STREQUAL "Debug") + set(ENABLE_COVERAGE ON) else() set(ENABLE_COVERAGE OFF) endif() From 871c761301ce8e6cbd96814921a1612ca0bcbdae Mon Sep 17 00:00:00 2001 From: Sergio Date: Fri, 26 Nov 2021 16:00:55 +0100 Subject: [PATCH 06/16] Clean-up a bit the temp. change by Nico --- include/bbp/sonata/report_reader.h | 2 +- src/report_reader.cpp | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/include/bbp/sonata/report_reader.h b/include/bbp/sonata/report_reader.h index f55f8253..b8a93ce0 100644 --- a/include/bbp/sonata/report_reader.h +++ b/include/bbp/sonata/report_reader.h @@ -133,7 +133,7 @@ class SONATA_API ReportReader */ typename DataFrame::DataType getIds( const nonstd::optional& node_ids = nonstd::nullopt, - std::function = nullptr) const; + std::function fn = [](...) {}) const; /** * \param node_ids limit the report to the given selection. diff --git a/src/report_reader.cpp b/src/report_reader.cpp index 78457f11..b50da227 100644 --- a/src/report_reader.cpp +++ b/src/report_reader.cpp @@ -341,10 +341,9 @@ std::pair ReportReader::Population::getIndex( template typename DataFrame::DataType ReportReader::Population::getIds( - const nonstd::optional& selection, std::function fun) const { + const nonstd::optional& selection, std::function fn) const { typename DataFrame::DataType ids{}; - // Simplify selection // We should remove duplicates // And when we can work with ranges let's sort them @@ -365,8 +364,8 @@ typename DataFrame::DataType ReportReader::Population::getIds( ids.push_back(make_key(node_id, elem)); } - if (fun) - fun(it->second); + // Call auxiliary function + fn(it->second); } return ids; } @@ -397,7 +396,7 @@ DataFrame ReportReader::Population::get(const nonstd::optional& Ranges positions; uint64_t min = std::numeric_limits::max(); uint64_t max = std::numeric_limits::min(); - data_frame.ids = getIds(selection, [&](const Range& range) { + data_frame.ids = getIds(selection, [&min, &max, &positions](const Range& range) { min = std::min(range.first, min); max = std::max(range.second, max); positions.emplace_back(range.first, range.second); From 3dd7b145992f626754f148a658eb41094a3f40b0 Mon Sep 17 00:00:00 2001 From: Sergio Date: Fri, 26 Nov 2021 17:22:30 +0100 Subject: [PATCH 07/16] Revert "Clean-up a bit the temp. change by Nico" This reverts commit 871c761301ce8e6cbd96814921a1612ca0bcbdae. --- include/bbp/sonata/report_reader.h | 2 +- src/report_reader.cpp | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/include/bbp/sonata/report_reader.h b/include/bbp/sonata/report_reader.h index b8a93ce0..f55f8253 100644 --- a/include/bbp/sonata/report_reader.h +++ b/include/bbp/sonata/report_reader.h @@ -133,7 +133,7 @@ class SONATA_API ReportReader */ typename DataFrame::DataType getIds( const nonstd::optional& node_ids = nonstd::nullopt, - std::function fn = [](...) {}) const; + std::function = nullptr) const; /** * \param node_ids limit the report to the given selection. diff --git a/src/report_reader.cpp b/src/report_reader.cpp index b50da227..78457f11 100644 --- a/src/report_reader.cpp +++ b/src/report_reader.cpp @@ -341,9 +341,10 @@ std::pair ReportReader::Population::getIndex( template typename DataFrame::DataType ReportReader::Population::getIds( - const nonstd::optional& selection, std::function fn) const { + const nonstd::optional& selection, std::function fun) const { typename DataFrame::DataType ids{}; + // Simplify selection // We should remove duplicates // And when we can work with ranges let's sort them @@ -364,8 +365,8 @@ typename DataFrame::DataType ReportReader::Population::getIds( ids.push_back(make_key(node_id, elem)); } - // Call auxiliary function - fn(it->second); + if (fun) + fun(it->second); } return ids; } @@ -396,7 +397,7 @@ DataFrame ReportReader::Population::get(const nonstd::optional& Ranges positions; uint64_t min = std::numeric_limits::max(); uint64_t max = std::numeric_limits::min(); - data_frame.ids = getIds(selection, [&min, &max, &positions](const Range& range) { + data_frame.ids = getIds(selection, [&](const Range& range) { min = std::min(range.first, min); max = std::max(range.second, max); positions.emplace_back(range.first, range.second); From 55ba079126913ff0d03aa01e3ebe91af9426c18e Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Fri, 26 Nov 2021 17:41:54 +0100 Subject: [PATCH 08/16] fun => fn --- src/report_reader.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/report_reader.cpp b/src/report_reader.cpp index 78457f11..e6b5e967 100644 --- a/src/report_reader.cpp +++ b/src/report_reader.cpp @@ -341,7 +341,7 @@ std::pair ReportReader::Population::getIndex( template typename DataFrame::DataType ReportReader::Population::getIds( - const nonstd::optional& selection, std::function fun) const { + const nonstd::optional& selection, std::function fn) const { typename DataFrame::DataType ids{}; @@ -365,8 +365,9 @@ typename DataFrame::DataType ReportReader::Population::getIds( ids.push_back(make_key(node_id, elem)); } - if (fun) - fun(it->second); + if (fn) { + fn(it->second); + } } return ids; } From 11a21914b2c68f9b1b1092738e85aef74d2eb815 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Wed, 1 Dec 2021 11:34:46 +0100 Subject: [PATCH 09/16] Remlove useless comments --- src/report_reader.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/report_reader.cpp b/src/report_reader.cpp index 109c8b4d..381ba538 100644 --- a/src/report_reader.cpp +++ b/src/report_reader.cpp @@ -286,10 +286,6 @@ std::vector ReportReader::Population::getNodeIds() const { template Selection::Values ReportReader::Population::node_ids_from_selection( const nonstd::optional& selection) const { - // Simplify selection - // We should remove duplicates - // And when we can work with ranges let's sort them - // auto nodes_ids_ = Selection::fromValues(node_ids.flatten().sort()); Selection::Values node_ids; if (!selection) { // Take all nodes in this case @@ -348,11 +344,6 @@ typename DataFrame::DataType ReportReader::Population::getIds( const nonstd::optional& selection, std::function fn) const { typename DataFrame::DataType ids{}; - - // Simplify selection - // We should remove duplicates - // And when we can work with ranges let's sort them - // auto nodes_ids_ = Selection::fromValues(node_ids.flatten().sort()); Selection::Values node_ids = node_ids_from_selection(selection); auto dataset_elem_ids = pop_group_.getGroup("mapping").getDataSet("element_ids"); From 0e15790c54c8c1d6c339151757daa8a2b175da0c Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Fri, 3 Dec 2021 15:49:54 +0100 Subject: [PATCH 10/16] Add cpp test and python bindings --- python/bindings.cpp | 3 +++ tests/test_report_reader.cpp | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/python/bindings.cpp b/python/bindings.cpp index 25b5d163..9cb06958 100644 --- a/python/bindings.cpp +++ b/python/bindings.cpp @@ -364,6 +364,9 @@ void bindReportReader(py::module& m, const std::string& prefix) { .def("get_node_ids", &ReportType::Population::getNodeIds, "Return the list of nodes ids for this population") + .def("get_ids", + &ReportType::Population::getIds, + "Return selected ids") .def_property_readonly("sorted", &ReportType::Population::getSorted, DOC_REPORTREADER_POP(getSorted)) diff --git a/tests/test_report_reader.cpp b/tests/test_report_reader.cpp index ce881a41..f3abe7b4 100644 --- a/tests/test_report_reader.cpp +++ b/tests/test_report_reader.cpp @@ -94,6 +94,9 @@ TEST_CASE("SomaReportReader", "[base]") { auto data_empty = pop.get(Selection({})); REQUIRE(data_empty.data == std::vector{}); + + auto ids = pop.getIds(Selection({{3, 5}})); + REQUIRE(ids == std::vector{3, 4}); } TEST_CASE("ElementReportReader limits", "[base]") { @@ -155,4 +158,7 @@ TEST_CASE("ElementReportReader", "[base]") { // Select only one time REQUIRE(pop.get(Selection({{1, 2}}), 0.6, 0.6).data == std::vector{30.0f, 30.1f, 30.2f, 30.3f, 30.4f}); + + auto ids = pop.getIds(Selection({{3, 5}})); + REQUIRE(ids == std::vector{{3, 5}, {3, 5}, {3, 6}, {3, 6}, {3, 7}, {4, 7}, {4, 8}, {4, 8}, {4, 9}, {4, 9}}); } From 6312289a73cd49da2841022677fcbac4a2ba1d0f Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Fri, 3 Dec 2021 16:03:17 +0100 Subject: [PATCH 11/16] clang-format --- python/bindings.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/python/bindings.cpp b/python/bindings.cpp index 9cb06958..258d0988 100644 --- a/python/bindings.cpp +++ b/python/bindings.cpp @@ -364,9 +364,7 @@ void bindReportReader(py::module& m, const std::string& prefix) { .def("get_node_ids", &ReportType::Population::getNodeIds, "Return the list of nodes ids for this population") - .def("get_ids", - &ReportType::Population::getIds, - "Return selected ids") + .def("get_ids", &ReportType::Population::getIds, "Return selected ids") .def_property_readonly("sorted", &ReportType::Population::getSorted, DOC_REPORTREADER_POP(getSorted)) From f7e0bfdd249281713c1c4c764ee30c6afe9cc641 Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Mon, 6 Dec 2021 09:31:55 +0100 Subject: [PATCH 12/16] fix python bindings --- python/bindings.cpp | 9 ++++++++- python/tests/test.py | 9 +++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/python/bindings.cpp b/python/bindings.cpp index 258d0988..a6d01fc0 100644 --- a/python/bindings.cpp +++ b/python/bindings.cpp @@ -364,7 +364,14 @@ void bindReportReader(py::module& m, const std::string& prefix) { .def("get_node_ids", &ReportType::Population::getNodeIds, "Return the list of nodes ids for this population") - .def("get_ids", &ReportType::Population::getIds, "Return selected ids") + .def( + "get_ids", + [](const typename ReportType::Population& population, + const nonstd::optional& selection) { + return population.getIds(selection, nullptr); + }, + DOC_REPORTREADER_POP(getIds), + "selection"_a = nonstd::nullopt) .def_property_readonly("sorted", &ReportType::Population::getSorted, DOC_REPORTREADER_POP(getSorted)) diff --git a/python/tests/test.py b/python/tests/test.py index 4b9f480c..fbeafaa9 100644 --- a/python/tests/test.py +++ b/python/tests/test.py @@ -294,6 +294,7 @@ def test_get_spikes_from_population(self): def test_getTimes_from_population(self): self.assertEqual(self.test_obj['All'].times, (0.1, 1.3)) + class TestSomaReportPopulation(unittest.TestCase): def setUp(self): path = os.path.join(PATH, "somas.h5") @@ -332,6 +333,10 @@ def test_get_reports_from_population(self): sel_empty = self.test_obj['All'].get(node_ids=[]) np.testing.assert_allclose(sel_empty.data, np.empty(shape=(0, 0))) + def test_get_ids(self): + self.assertEqual(self.test_obj['All'].get_ids([[3, 5]]), + [3, 4]) + class TestElementReportPopulation(unittest.TestCase): def setUp(self): @@ -384,6 +389,10 @@ def test_get_reports_from_population(self): np.testing.assert_allclose(self.test_obj['All'].get(node_ids=[3, 4], tstride=4).data[2], [81.0, 81.1, 81.2, 81.3, 81.4, 81.5, 81.6, 81.7, 81.8, 81.9], 1e-6, 0) + def test_get_ids(self): + self.assertEqual(self.test_obj['All'].get_ids([[3, 5]]), + [[3, 5], [3, 5], [3, 6], [3, 6], [3, 7], [4, 7], [4, 8], [4, 8], [4, 9], [4, 9]]) + class TestNodePopulationFailure(unittest.TestCase): def test_CorrectStructure(self): From 79153c562ebb84cbf8dccfe92bc1e7148b66b311 Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Tue, 7 Dec 2021 09:26:12 +0100 Subject: [PATCH 13/16] getIds -> getNodeIdElementIdMapping, improved doc --- include/bbp/sonata/report_reader.h | 9 ++++++--- python/bindings.cpp | 6 +++--- python/generated/docstrings.h | 2 +- python/tests/test.py | 8 ++++---- src/report_reader.cpp | 4 ++-- tests/test_report_reader.cpp | 4 ++-- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/include/bbp/sonata/report_reader.h b/include/bbp/sonata/report_reader.h index 434f871e..02f6b3a0 100644 --- a/include/bbp/sonata/report_reader.h +++ b/include/bbp/sonata/report_reader.h @@ -134,11 +134,14 @@ class SONATA_API ReportReader std::vector getNodeIds() const; /** - * Return selected ids. + * Return the ElementIds for the passed Node + * + * \param node_ids limit the report to the given selection. If nullptr, all nodes are used + * \param fn lambda applied to all ranges for all node ids */ - typename DataFrame::DataType getIds( + typename DataFrame::DataType getNodeIdElementIdMapping( const nonstd::optional& node_ids = nonstd::nullopt, - std::function = nullptr) const; + std::function fn = nullptr) const; /** * \param node_ids limit the report to the given selection. diff --git a/python/bindings.cpp b/python/bindings.cpp index a6d01fc0..40514f9b 100644 --- a/python/bindings.cpp +++ b/python/bindings.cpp @@ -365,12 +365,12 @@ void bindReportReader(py::module& m, const std::string& prefix) { &ReportType::Population::getNodeIds, "Return the list of nodes ids for this population") .def( - "get_ids", + "get_node_id_element_id_mapping", [](const typename ReportType::Population& population, const nonstd::optional& selection) { - return population.getIds(selection, nullptr); + return population.getNodeIdElementIdMapping(selection, nullptr); }, - DOC_REPORTREADER_POP(getIds), + DOC_REPORTREADER_POP(getNodeIdElementIdMapping), "selection"_a = nonstd::nullopt) .def_property_readonly("sorted", &ReportType::Population::getSorted, diff --git a/python/generated/docstrings.h b/python/generated/docstrings.h index e0a95cfa..86a52dd7 100644 --- a/python/generated/docstrings.h +++ b/python/generated/docstrings.h @@ -429,7 +429,7 @@ Parameter ``tstride``: static const char *__doc_bbp_sonata_ReportReader_Population_getDataUnits = R"doc(Return the unit of data.)doc"; -static const char *__doc_bbp_sonata_ReportReader_Population_getIds = R"doc(Return selected ids.)doc"; +static const char *__doc_bbp_sonata_ReportReader_Population_getNodeIdElementIdMapping = R"doc(Return selected ids.)doc"; static const char *__doc_bbp_sonata_ReportReader_Population_getIndex = R"doc()doc"; diff --git a/python/tests/test.py b/python/tests/test.py index fbeafaa9..30fb3b9e 100644 --- a/python/tests/test.py +++ b/python/tests/test.py @@ -333,8 +333,8 @@ def test_get_reports_from_population(self): sel_empty = self.test_obj['All'].get(node_ids=[]) np.testing.assert_allclose(sel_empty.data, np.empty(shape=(0, 0))) - def test_get_ids(self): - self.assertEqual(self.test_obj['All'].get_ids([[3, 5]]), + def test_get_node_id_element_id_mapping(self): + self.assertEqual(self.test_obj['All'].get_node_id_element_id_mapping([[3, 5]]), [3, 4]) @@ -389,8 +389,8 @@ def test_get_reports_from_population(self): np.testing.assert_allclose(self.test_obj['All'].get(node_ids=[3, 4], tstride=4).data[2], [81.0, 81.1, 81.2, 81.3, 81.4, 81.5, 81.6, 81.7, 81.8, 81.9], 1e-6, 0) - def test_get_ids(self): - self.assertEqual(self.test_obj['All'].get_ids([[3, 5]]), + def test_get_node_id_element_id_mapping(self): + self.assertEqual(self.test_obj['All'].get_node_id_element_id_mapping([[3, 5]]), [[3, 5], [3, 5], [3, 6], [3, 6], [3, 7], [4, 7], [4, 8], [4, 8], [4, 9], [4, 9]]) diff --git a/src/report_reader.cpp b/src/report_reader.cpp index 381ba538..3723c7b2 100644 --- a/src/report_reader.cpp +++ b/src/report_reader.cpp @@ -340,7 +340,7 @@ std::pair ReportReader::Population::getIndex( template -typename DataFrame::DataType ReportReader::Population::getIds( +typename DataFrame::DataType ReportReader::Population::getNodeIdElementIdMapping( const nonstd::optional& selection, std::function fn) const { typename DataFrame::DataType ids{}; @@ -393,7 +393,7 @@ DataFrame ReportReader::Population::get(const nonstd::optional& Ranges positions; uint64_t min = std::numeric_limits::max(); uint64_t max = std::numeric_limits::min(); - data_frame.ids = getIds(selection, [&](const Range& range) { + data_frame.ids = getNodeIdElementIdMapping(selection, [&](const Range& range) { min = std::min(range.first, min); max = std::max(range.second, max); positions.emplace_back(range.first, range.second); diff --git a/tests/test_report_reader.cpp b/tests/test_report_reader.cpp index f3abe7b4..16c73283 100644 --- a/tests/test_report_reader.cpp +++ b/tests/test_report_reader.cpp @@ -95,7 +95,7 @@ TEST_CASE("SomaReportReader", "[base]") { auto data_empty = pop.get(Selection({})); REQUIRE(data_empty.data == std::vector{}); - auto ids = pop.getIds(Selection({{3, 5}})); + auto ids = pop.getNodeIdElementIdMapping(Selection({{3, 5}})); REQUIRE(ids == std::vector{3, 4}); } @@ -159,6 +159,6 @@ TEST_CASE("ElementReportReader", "[base]") { REQUIRE(pop.get(Selection({{1, 2}}), 0.6, 0.6).data == std::vector{30.0f, 30.1f, 30.2f, 30.3f, 30.4f}); - auto ids = pop.getIds(Selection({{3, 5}})); + auto ids = pop.getNodeIdElementIdMapping(Selection({{3, 5}})); REQUIRE(ids == std::vector{{3, 5}, {3, 5}, {3, 6}, {3, 6}, {3, 7}, {4, 7}, {4, 8}, {4, 8}, {4, 9}, {4, 9}}); } From a0f3a0214bcbd8f9e92c3dedc0a398dc19e1ef5a Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Tue, 7 Dec 2021 09:33:47 +0100 Subject: [PATCH 14/16] fix docstrings --- python/generated/docstrings.h | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/python/generated/docstrings.h b/python/generated/docstrings.h index 86a52dd7..b87ca9e0 100644 --- a/python/generated/docstrings.h +++ b/python/generated/docstrings.h @@ -429,10 +429,18 @@ Parameter ``tstride``: static const char *__doc_bbp_sonata_ReportReader_Population_getDataUnits = R"doc(Return the unit of data.)doc"; -static const char *__doc_bbp_sonata_ReportReader_Population_getNodeIdElementIdMapping = R"doc(Return selected ids.)doc"; - static const char *__doc_bbp_sonata_ReportReader_Population_getIndex = R"doc()doc"; +static const char *__doc_bbp_sonata_ReportReader_Population_getNodeIdElementIdMapping = +R"doc(Return the ElementIds for the passed Node + +Parameter ``node_ids``: + limit the report to the given selection. If nullptr, all nodes are + used + +Parameter ``fn``: + lambda applied to all ranges for all node ids)doc"; + static const char *__doc_bbp_sonata_ReportReader_Population_getNodeIds = R"doc(Return all the node ids.)doc"; static const char *__doc_bbp_sonata_ReportReader_Population_getSorted = R"doc(Return true if the data is sorted.)doc"; From f5ba564987e8a21acc0bf338185d703183fc32a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nadir=20Rom=C3=A1n=20Guerrero?= Date: Tue, 14 Dec 2021 16:01:52 +0100 Subject: [PATCH 15/16] Added extra doc for getNodeIdElementIdMapping() --- include/bbp/sonata/report_reader.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/include/bbp/sonata/report_reader.h b/include/bbp/sonata/report_reader.h index 02f6b3a0..0fa7c255 100644 --- a/include/bbp/sonata/report_reader.h +++ b/include/bbp/sonata/report_reader.h @@ -134,9 +134,16 @@ class SONATA_API ReportReader std::vector getNodeIds() const; /** - * Return the ElementIds for the passed Node + * Return the ElementIds for the passed Node. + * The return type will depend on the report reader: + * - For Soma report reader, the return value will be the Node ID to which the report + * value belongs to. + * - For Element/full compartment readers, the return value will be an array with 2 + * elements, the first element is the Node ID and the second element is the + * compartment ID of the given Node. * - * \param node_ids limit the report to the given selection. If nullptr, all nodes are used + * \param node_ids limit the report to the given selection. If nullptr, all nodes in the + * report are used * \param fn lambda applied to all ranges for all node ids */ typename DataFrame::DataType getNodeIdElementIdMapping( From bbf448fee0d43b7c7fd0dfde4cf0bd0da8a3ab9e Mon Sep 17 00:00:00 2001 From: Mike Gevaert Date: Wed, 15 Dec 2021 09:01:31 +0100 Subject: [PATCH 16/16] fix docs --- python/generated/docstrings.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/python/generated/docstrings.h b/python/generated/docstrings.h index aa7c608f..e7d690f3 100644 --- a/python/generated/docstrings.h +++ b/python/generated/docstrings.h @@ -432,11 +432,16 @@ static const char *__doc_bbp_sonata_ReportReader_Population_getDataUnits = R"doc static const char *__doc_bbp_sonata_ReportReader_Population_getIndex = R"doc()doc"; static const char *__doc_bbp_sonata_ReportReader_Population_getNodeIdElementIdMapping = -R"doc(Return the ElementIds for the passed Node +R"doc(Return the ElementIds for the passed Node. The return type will depend +on the report reader: - For Soma report reader, the return value will +be the Node ID to which the report value belongs to. - For +Element/full compartment readers, the return value will be an array +with 2 elements, the first element is the Node ID and the second +element is the compartment ID of the given Node. Parameter ``node_ids``: - limit the report to the given selection. If nullptr, all nodes are - used + limit the report to the given selection. If nullptr, all nodes in + the report are used Parameter ``fn``: lambda applied to all ranges for all node ids)doc";