From 9ba907f9f79516aa8906b09384e4925ec96d04b0 Mon Sep 17 00:00:00 2001 From: Dhanya Thattil <33750417+thattil@users.noreply.github.com> Date: Fri, 27 Jan 2023 09:59:31 +0100 Subject: [PATCH] added none or 0 to unset bad channels (#632) * added none or 0 to unset bad channels * free function split to get int array from string of arguments for badchannels * missed a file * allowing list for badchannels in command line * added badchannels in python * added size check * more comments in Detector.h and added more tests for facny command line badchannels * removeDuplicates accept any container, added tests * corner cases: 1:5,6,7 or 5,6,7 or just 1:5 Co-authored-by: Erik Frojdh --- python/slsdet/detector.py | 13 ++ python/src/detector.cpp | 15 +++ slsDetectorSoftware/include/sls/Detector.h | 11 ++ slsDetectorSoftware/src/CmdProxy.cpp | 26 +++- slsDetectorSoftware/src/Detector.cpp | 23 ++++ slsDetectorSoftware/src/DetectorImpl.cpp | 4 + slsDetectorSoftware/src/DetectorImpl.h | 5 +- slsDetectorSoftware/tests/test-CmdProxy.cpp | 47 +++++++ slsSupportLib/include/sls/container_utils.h | 12 ++ slsSupportLib/include/sls/file_utils.h | 2 + slsSupportLib/src/file_utils.cpp | 124 ++++++++++--------- slsSupportLib/tests/test-container_utils.cpp | 15 +++ 12 files changed, 231 insertions(+), 66 deletions(-) diff --git a/python/slsdet/detector.py b/python/slsdet/detector.py index 6ca17308c..1692e3148 100755 --- a/python/slsdet/detector.py +++ b/python/slsdet/detector.py @@ -1604,6 +1604,19 @@ class Detector(CppDetectorApi): def sync(self, value): ut.set_using_dict(self.setSynchronization, value) + @property + @element + def badchannels(self): + """ + [fname|none|0]\n\t[Gotthard2][Mythen3] Sets the bad channels (from file of bad channel numbers) to be masked out. None or 0 unsets all the badchannels.\n + [Mythen3] Also does trimming + """ + return self.getBadChannels() + + @badchannels.setter + def badchannels(self, value): + ut.set_using_dict(self.setBadChannels, value) + @property @element def lock(self): diff --git a/python/src/detector.cpp b/python/src/detector.cpp index 0df2feaef..6256252d7 100644 --- a/python/src/detector.cpp +++ b/python/src/detector.cpp @@ -219,6 +219,21 @@ void init_det(py::module &m) { (void (Detector::*)(const std::string &, sls::Positions)) & Detector::setBadChannels, py::arg(), py::arg() = Positions{}); + CppDetectorApi.def( + "getBadChannels", + (Result>(Detector::*)(sls::Positions) const) & + Detector::getBadChannels, + py::arg() = Positions{}); + CppDetectorApi.def( + "setBadChannels", + (void (Detector::*)(const std::vector, sls::Positions)) & + Detector::setBadChannels, + py::arg(), py::arg() = Positions{}); + CppDetectorApi.def( + "setBadChannels", + (void (Detector::*)(const std::vector>)) & + Detector::setBadChannels, + py::arg()); CppDetectorApi.def("isVirtualDetectorServer", (Result(Detector::*)(sls::Positions) const) & Detector::isVirtualDetectorServer, diff --git a/slsDetectorSoftware/include/sls/Detector.h b/slsDetectorSoftware/include/sls/Detector.h index 248607ca1..ea17f9c78 100644 --- a/slsDetectorSoftware/include/sls/Detector.h +++ b/slsDetectorSoftware/include/sls/Detector.h @@ -221,6 +221,17 @@ class Detector { */ void setBadChannels(const std::string &fname, Positions pos = {}); + /** [Gotthard2][Mythen3] */ + Result> getBadChannels(Positions pos = {}) const; + + /** [Gotthard2][Mythen3] Empty list resets bad channel list */ + void setBadChannels(const std::vector list, Positions pos = {}); + + /** [Gotthard2][Mythen3] Size of list should match number of modules. Each + * value is at module level and can start at 0. Empty vector resets bad + * channel list. */ + void setBadChannels(const std::vector> list); + Result isVirtualDetectorServer(Positions pos = {}) const; ///@} diff --git a/slsDetectorSoftware/src/CmdProxy.cpp b/slsDetectorSoftware/src/CmdProxy.cpp index 1a36e7f3a..f222c56fd 100644 --- a/slsDetectorSoftware/src/CmdProxy.cpp +++ b/slsDetectorSoftware/src/CmdProxy.cpp @@ -7,6 +7,7 @@ #include "sls/ToString.h" #include "sls/bit_utils.h" #include "sls/container_utils.h" +#include "sls/file_utils.h" #include "sls/logger.h" #include "sls/sls_detector_defs.h" @@ -548,9 +549,9 @@ std::string CmdProxy::BadChannels(int action) { std::ostringstream os; os << cmd << ' '; if (action == defs::HELP_ACTION) { - os << "[fname]\n\t[Gotthard2][Mythen3] Sets the bad channels (from " - "file of bad channel numbers) to be masked out." - "\n\t[Mythen3] Also does trimming" + os << "[fname|none|0]\n\t[Gotthard2][Mythen3] Sets the bad channels " + "(from file of bad channel numbers) to be masked out. None or 0 " + "unsets all the badchannels.\n\t[Mythen3] Also does trimming" << '\n'; } else if (action == defs::GET_ACTION) { if (args.size() != 1) { @@ -559,10 +560,25 @@ std::string CmdProxy::BadChannels(int action) { det->getBadChannels(args[0], std::vector{det_id}); os << "successfully retrieved" << '\n'; } else if (action == defs::PUT_ACTION) { - if (args.size() != 1) { + bool parse = false; + if (args.size() == 0) { WrongNumberOfParameters(1); + } else if (args.size() == 1) { + if (args[0] == "none" || args[0] == "0") { + det->setBadChannels(std::vector{}, + std::vector{det_id}); + } else if (args[0].find(".") != std::string::npos) { + det->setBadChannels(args[0], std::vector{det_id}); + } else { + parse = true; + } + } + // parse multi args or single one with range or single value + if (parse || args.size() > 1) { + // get channels + auto list = getChannelsFromStringList(args); + det->setBadChannels(list, std::vector{det_id}); } - det->setBadChannels(args[0], std::vector{det_id}); os << "successfully loaded" << '\n'; } else { throw RuntimeError("Unknown action"); diff --git a/slsDetectorSoftware/src/Detector.cpp b/slsDetectorSoftware/src/Detector.cpp index 343fa283f..0692b0d09 100644 --- a/slsDetectorSoftware/src/Detector.cpp +++ b/slsDetectorSoftware/src/Detector.cpp @@ -349,6 +349,29 @@ void Detector::setBadChannels(const std::string &fname, Positions pos) { pimpl->setBadChannels(fname, pos); } +Result> Detector::getBadChannels(Positions pos) const { + return pimpl->Parallel(&Module::getBadChannels, pos); +} + +void Detector::setBadChannels(const std::vector> list) { + + if (list.size() != static_cast(size())) { + std::stringstream ss; + ss << "Number of bad channel sets (" << list.size() + << ") needs to match the number of modules (" << size() << ")"; + throw RuntimeError(ss.str()); + } + + for (int idet = 0; idet < size(); ++idet) { + // TODO! Call in parallel since loading trimbits is slow? + pimpl->Parallel(&Module::setBadChannels, {idet}, list[idet]); + } +} + +void Detector::setBadChannels(const std::vector list, Positions pos) { + pimpl->setBadChannels(list, pos); +} + Result Detector::isVirtualDetectorServer(Positions pos) const { return pimpl->Parallel(&Module::isVirtualDetectorServer, pos); } diff --git a/slsDetectorSoftware/src/DetectorImpl.cpp b/slsDetectorSoftware/src/DetectorImpl.cpp index e28410f91..ffa8c4275 100644 --- a/slsDetectorSoftware/src/DetectorImpl.cpp +++ b/slsDetectorSoftware/src/DetectorImpl.cpp @@ -1778,6 +1778,10 @@ void DetectorImpl::setBadChannels(const std::string &fname, Positions pos) { if (list.empty()) { throw RuntimeError("Bad channel file is empty."); } + setBadChannels(list, pos); +} + +void DetectorImpl::setBadChannels(const std::vector list, Positions pos) { // update to multi values if multi modules if (isAllPositions(pos)) { diff --git a/slsDetectorSoftware/src/DetectorImpl.h b/slsDetectorSoftware/src/DetectorImpl.h index 11fde7235..7de70b753 100644 --- a/slsDetectorSoftware/src/DetectorImpl.h +++ b/slsDetectorSoftware/src/DetectorImpl.h @@ -84,7 +84,9 @@ class DetectorImpl : public virtual slsDetectorDefs { */ virtual ~DetectorImpl(); - template struct NonDeduced { using type = CT; }; + template struct NonDeduced { + using type = CT; + }; template Result Parallel(RT (Module::*somefunc)(CT...), std::vector positions, @@ -306,6 +308,7 @@ class DetectorImpl : public virtual slsDetectorDefs { void getBadChannels(const std::string &fname, Positions pos) const; void setBadChannels(const std::string &fname, Positions pos); + void setBadChannels(const std::vector list, Positions pos); std::vector getCtbDacNames() const; std::string getCtbDacName(defs::dacIndex i) const; diff --git a/slsDetectorSoftware/tests/test-CmdProxy.cpp b/slsDetectorSoftware/tests/test-CmdProxy.cpp index dcee45551..574877b41 100644 --- a/slsDetectorSoftware/tests/test-CmdProxy.cpp +++ b/slsDetectorSoftware/tests/test-CmdProxy.cpp @@ -644,6 +644,8 @@ TEST_CASE("badchannels", "[.cmd]") { auto det_type = det.getDetectorType().squash(); if (det_type == defs::GOTTHARD2 || det_type == defs::MYTHEN3) { + auto prev = det.getBadChannels(); + REQUIRE_THROWS(proxy.Call("badchannels", {}, -1, GET)); std::string fname_put = @@ -656,6 +658,51 @@ TEST_CASE("badchannels", "[.cmd]") { std::vector expected = {0, 12, 15, 40, 41, 42, 43, 44, 1279}; REQUIRE(list == expected); + REQUIRE_NOTHROW(proxy.Call("badchannels", {"none"}, 0, PUT)); + REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_get}, 0, GET)); + list = getChannelsFromFile(fname_get); + REQUIRE(list.empty()); + + REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_put}, 0, PUT)); + + REQUIRE_NOTHROW(proxy.Call("badchannels", {"0"}, 0, PUT)); + REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_get}, 0, GET)); + list = getChannelsFromFile(fname_get); + REQUIRE(list.empty()); + + REQUIRE_NOTHROW(proxy.Call("badchannels", {"12"}, 0, PUT)); + REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_get}, 0, GET)); + list = getChannelsFromFile(fname_get); + expected = {12}; + REQUIRE(list == expected); + + REQUIRE_NOTHROW(proxy.Call( + "badchannels", {"0", "12,", "15", "43", "40:45", "1279"}, 0, PUT)); + REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_get}, 0, GET)); + list = getChannelsFromFile(fname_get); + expected = {0, 12, 15, 40, 41, 42, 43, 44, 1279}; + REQUIRE(list == expected); + + REQUIRE_NOTHROW(proxy.Call("badchannels", {"40:45"}, 0, PUT)); + REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_get}, 0, GET)); + list = getChannelsFromFile(fname_get); + expected = {40, 41, 42, 43, 44}; + REQUIRE(list == expected); + + REQUIRE_NOTHROW(proxy.Call("badchannels", {"5,6,7"}, 0, PUT)); + REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_get}, 0, GET)); + list = getChannelsFromFile(fname_get); + expected = {5, 6, 7}; + REQUIRE(list == expected); + + REQUIRE_NOTHROW(proxy.Call("badchannels", {"1:5,6,7"}, 0, PUT)); + REQUIRE_NOTHROW(proxy.Call("badchannels", {fname_get}, 0, GET)); + list = getChannelsFromFile(fname_get); + expected = {1, 2, 3, 4, 6, 7}; + REQUIRE(list == expected); + + det.setBadChannels(prev); + } else { REQUIRE_THROWS(proxy.Call("badchannels", {}, -1, GET)); } diff --git a/slsSupportLib/include/sls/container_utils.h b/slsSupportLib/include/sls/container_utils.h index 08fa534d6..650c05b7c 100644 --- a/slsSupportLib/include/sls/container_utils.h +++ b/slsSupportLib/include/sls/container_utils.h @@ -148,6 +148,18 @@ Squash(const Container &c, typename Container::value_type default_value = {}) { return default_value; } +template +typename std::enable_if::value, bool>::type +removeDuplicates(T &c) { + auto containerSize = c.size(); + std::sort(c.begin(), c.end()); + c.erase(std::unique(c.begin(), c.end()), c.end()); + if (c.size() != containerSize) { + return true; + } + return false; +} + } // namespace sls #endif // CONTAINER_UTILS_H diff --git a/slsSupportLib/include/sls/file_utils.h b/slsSupportLib/include/sls/file_utils.h index 5f0bc5967..010fc640c 100644 --- a/slsSupportLib/include/sls/file_utils.h +++ b/slsSupportLib/include/sls/file_utils.h @@ -50,6 +50,8 @@ ssize_t getFileSize(FILE *fd, const std::string &prependErrorString); std::string getFileNameFromFilePath(const std::string &fpath); +std::vector getChannelsFromStringList(const std::vector list); + /** File can have # for comments. * Channels can be separated by spaces, commas * and ranges provided using ':', eg. 23:29 diff --git a/slsSupportLib/src/file_utils.cpp b/slsSupportLib/src/file_utils.cpp index 36a28c853..15fb3a1c3 100644 --- a/slsSupportLib/src/file_utils.cpp +++ b/slsSupportLib/src/file_utils.cpp @@ -2,6 +2,7 @@ // Copyright (C) 2021 Contributors to the SLS Detector Package #include "sls/file_utils.h" #include "sls/ToString.h" +#include "sls/container_utils.h" #include "sls/logger.h" #include "sls/sls_detector_exceptions.h" @@ -165,6 +166,63 @@ ssize_t getFileSize(FILE *fd, const std::string &prependErrorString) { return fileSize; } +std::vector +getChannelsFromStringList(const std::vector list) { + std::vector channels; + for (auto line : list) { + + // replace comma with space + std::replace_if( + begin(line), end(line), [](char c) { return (c == ','); }, ' '); + + // split line (delim space) + std::vector vec = split(line, ' '); + + // for every channel separated by space + for (auto it : vec) { + // find range and replace with sequence of x to y + auto result = it.find(':'); + if (result != std::string::npos) { + try { + int istart = StringTo(it.substr(0, result)); + int istop = StringTo( + it.substr(result + 1, it.length() - result - 1)); + LOG(logDEBUG1) << "istart:" << istart << " istop:" << istop; + std::vector range(istop - istart); + std::generate(range.begin(), range.end(), + [n = istart]() mutable { return n++; }); + for (auto range_it : range) { + channels.push_back(range_it); + } + } catch (std::exception &e) { + throw RuntimeError( + "Could not load channels. Invalid channel range: " + + it); + } + } + + // else convert to int + else { + int ival = 0; + try { + ival = StringTo(it); + } catch (std::exception &e) { + throw RuntimeError( + "Could not load channels. Invalid channel number: " + + it); + } + channels.push_back(ival); + } + } + } + + if (removeDuplicates(channels)) { + LOG(logWARNING) << "Removed duplicates from channel file"; + } + LOG(logDEBUG1) << "list:" << ToString(channels); + return channels; +} + std::vector getChannelsFromFile(const std::string &fname) { // read bad channels file std::ifstream input_file(fname); @@ -172,73 +230,19 @@ std::vector getChannelsFromFile(const std::string &fname) { throw RuntimeError("Could not open bad channels file " + fname + " for reading"); } - std::vector list; + std::vector lines; for (std::string line; std::getline(input_file, line);) { // ignore comments if (line.find('#') != std::string::npos) { line.erase(line.find('#')); } - - // replace comma with space - std::replace_if( - begin(line), end(line), [](char c) { return (c == ','); }, ' '); - - // replace x:y with a sequence of x to y - auto result = line.find(':'); - while (result != std::string::npos) { - auto start = line.rfind(' ', result); - if (start == std::string::npos) { - start = 0; - } else - ++start; - int istart = StringTo(line.substr(start, result - start)); - - auto stop = line.find(' ', result); - if (stop == std::string::npos) { - stop = line.length(); - } - int istop = - StringTo(line.substr(result + 1, stop - result - 1)); - - std::vector v(istop - istart); - std::generate(v.begin(), v.end(), - [n = istart]() mutable { return n++; }); - line.replace(start, stop - start, ToString(v)); - - LOG(logDEBUG1) << line; - result = line.find(':'); - } - - // remove punctuations including [ and ] - line.erase(std::remove_if(begin(line), end(line), ispunct), end(line)); - - LOG(logDEBUG) << "\nline: [" << line << ']'; - - // split line (delim space) and push to list - std::vector vec = split(line, ' '); - for (auto it : vec) { - int ival = 0; - try { - ival = StringTo(it); - } catch (std::exception &e) { - throw RuntimeError("Could not load channels from file. Invalid " - "channel number: " + - it); - } - list.push_back(ival); + // ignore empty lines + if (line.empty()) { + continue; } + lines.push_back(line); } - - // remove duplicates from list - auto listSize = list.size(); - std::sort(list.begin(), list.end()); - list.erase(unique(list.begin(), list.end()), list.end()); - if (list.size() != listSize) { - LOG(logWARNING) << "Removed duplicates from channel file"; - } - - LOG(logDEBUG1) << "list:" << ToString(list); - return list; + return getChannelsFromStringList(lines); } std::string getAbsolutePathFromCurrentProcess(const std::string &fname) { diff --git a/slsSupportLib/tests/test-container_utils.cpp b/slsSupportLib/tests/test-container_utils.cpp index ea98c084a..061ee6e4e 100644 --- a/slsSupportLib/tests/test-container_utils.cpp +++ b/slsSupportLib/tests/test-container_utils.cpp @@ -136,4 +136,19 @@ TEST_CASE("compare a vector of arrays", "[support]") { CHECK(minusOneIfDifferent(vec1) == arr); } +TEST_CASE("remove duplicates from vector"){ + std::vector v{5,6,5,3}; + auto r = removeDuplicates(v); + CHECK(r == true); //did indeed remove elements + CHECK( v == std::vector{3,5,6}); +} + +TEST_CASE("remove duplicated empty vector"){ + std::vector v; + auto r = removeDuplicates(v); + CHECK(r == false); //no elements to remove + CHECK( v == std::vector{}); +} + + } // namespace sls