From c17ce551fac7f8053aaaa28e38760b290340995e Mon Sep 17 00:00:00 2001 From: Nicolau Manubens Date: Mon, 22 Jan 2024 20:34:54 +0000 Subject: [PATCH 01/17] Modifications for DAOS backend. --- src/eckit/filesystem/URI.h | 2 +- src/eckit/io/MultiHandle.cc | 6 ++++-- src/eckit/log/TimeStamp.cc | 7 +++++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/eckit/filesystem/URI.h b/src/eckit/filesystem/URI.h index 0cd9a4824..523d3e3f8 100644 --- a/src/eckit/filesystem/URI.h +++ b/src/eckit/filesystem/URI.h @@ -43,7 +43,7 @@ class URI { // Contructors URI(); - URI(const std::string& uri); + explicit URI(const std::string& uri); URI(const std::string& scheme, const PathName& path); URI(const std::string& scheme, const URI& uri); URI(const std::string& scheme, const std::string& hostname, int port); diff --git a/src/eckit/io/MultiHandle.cc b/src/eckit/io/MultiHandle.cc index 659fe5396..d634feef7 100644 --- a/src/eckit/io/MultiHandle.cc +++ b/src/eckit/io/MultiHandle.cc @@ -149,11 +149,13 @@ long MultiHandle::read1(char* buffer, long length) { } long n = (*current_)->read(buffer, length); - if (n <= 0) { + if (n < length) { (*current_)->close(); current_++; openCurrent(); - return read1(buffer, length); + if (n <= 0) { + return read1(buffer, length); + } } return n; } diff --git a/src/eckit/log/TimeStamp.cc b/src/eckit/log/TimeStamp.cc index 057d6a52a..0a2d36d20 100644 --- a/src/eckit/log/TimeStamp.cc +++ b/src/eckit/log/TimeStamp.cc @@ -9,6 +9,7 @@ */ #include +#include #include "eckit/eckit.h" @@ -27,6 +28,12 @@ TimeStamp::TimeStamp(time_t t, const std::string& format) : time_(t), format_(format) {} std::ostream& operator<<(std::ostream& s, const TimeStamp& x) { + + if (x.format_ == "hex") { + s << std::setw(16) << std::setfill('0') << std::hex << (uint64_t) x.time_; + return s; + } + char buf[80]; #if eckit_HAVE_GMTIME_R struct tm t; From 523d91c92557c2ed298a3a6fd1e77c86eecd7875 Mon Sep 17 00:00:00 2001 From: DJDavies2 Date: Tue, 26 Mar 2024 11:33:33 +0000 Subject: [PATCH 02/17] Correct install location for codec file. --- src/eckit/codec/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/eckit/codec/CMakeLists.txt b/src/eckit/codec/CMakeLists.txt index 4875895c2..46ec94676 100644 --- a/src/eckit/codec/CMakeLists.txt +++ b/src/eckit/codec/CMakeLists.txt @@ -2,7 +2,7 @@ configure_file( eckit_codec_config.h.in ${CMAKE_CURRENT_BINARY_DIR}/eckit_codec_config.h @ONLY ) install( FILES ${CMAKE_CURRENT_BINARY_DIR}/eckit_codec_config.h - DESTINATION ${INSTALL_INCLUDE_DIR}/eckit + DESTINATION ${INSTALL_INCLUDE_DIR}/eckit/codec ) ecbuild_add_library( From f45d23935755b12aeff42f2183a214835a80cb31 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Wed, 17 Apr 2024 13:00:47 +0200 Subject: [PATCH 03/17] Make experimental CUDA feature DEFAULT OFF (fixes ecmwf-toolbox) --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 549ec3bd8..de73cb552 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -211,6 +211,7 @@ ecbuild_add_option( FEATURE JEMALLOC #### CUDA ecbuild_add_option( FEATURE CUDA + DEFAULT OFF CONDITION HAVE_EXPERIMENTAL DESCRIPTION "CUDA GPU linear algebra operations" REQUIRED_PACKAGES CUDA ) From 28e913ab6dbb677202f5abcde60ae8864257b0f6 Mon Sep 17 00:00:00 2001 From: Iain Russell <40060766+iainrussell@users.noreply.github.com> Date: Mon, 29 Apr 2024 08:29:47 +0100 Subject: [PATCH 04/17] Update README.md - trivial whitespace change to trigger ci --- README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/README.md b/README.md index 1850a0884..424338622 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,3 @@ make install # 4. Check installation $installdir/bin/eckit-version ``` - - - From 1f5a5af040faa71499e5e38cc3f4c8a4f94ab43c Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Tue, 23 Apr 2024 17:56:53 +0200 Subject: [PATCH 05/17] Use std::string_view in eckit_mpi --- src/eckit/mpi/Comm.cc | 82 ++++++++++++++++---------------- src/eckit/mpi/Comm.h | 31 ++++++------ src/eckit/mpi/Parallel.cc | 8 ++-- src/eckit/mpi/Parallel.h | 6 +-- src/eckit/mpi/ParallelGroup.cc | 7 +-- src/eckit/mpi/ParallelRequest.cc | 4 +- src/eckit/mpi/Serial.cc | 4 +- src/eckit/mpi/Serial.h | 4 +- 8 files changed, 75 insertions(+), 71 deletions(-) diff --git a/src/eckit/mpi/Comm.cc b/src/eckit/mpi/Comm.cc index eb88fa252..6071135fb 100644 --- a/src/eckit/mpi/Comm.cc +++ b/src/eckit/mpi/Comm.cc @@ -36,7 +36,7 @@ constexpr bool have_parallel() { class Environment { public: - static const char* getDefaultCommType() { + static std::string_view getDefaultCommType() { // Force a given communicator (only required if e.g. running serial applications with MPI) if (const char* forcedComm = ::getenv("ECKIT_MPI_FORCE")) { return forcedComm; @@ -80,10 +80,10 @@ class Environment { default_ = world; } - void setDefaut(const char* name) { + void setDefault(std::string_view name) { AutoLock lock(mutex_); - std::map::iterator itr = communicators.find(name); + auto itr = communicators.find(name); if (itr != communicators.end()) { default_ = itr->second; return; @@ -95,10 +95,10 @@ class Environment { for (itr = communicators.begin(); itr != communicators.end(); ++itr) { eckit::Log::error() << " " << (*itr).first << std::endl; } - throw eckit::SeriousBug(std::string("No communicator called ") + name, Here()); + throw eckit::SeriousBug("No communicator called " + std::string{name}, Here()); } - bool hasComm(const char* name) { + bool hasComm(std::string_view name) { AutoLock lock(mutex_); std::map::iterator itr = communicators.find(name); if (itr != communicators.end()) { @@ -120,17 +120,17 @@ class Environment { void finaliseAllComms() { AutoLock lock(mutex_); - std::map::iterator itr = communicators.begin(); + auto itr = communicators.begin(); for (; itr != communicators.end(); ++itr) { delete itr->second; } communicators.clear(); } - Comm& getComm(const char* name = nullptr) { + Comm& getComm(std::string_view name = {}) { AutoLock lock(mutex_); - if (!name && default_) { + if (name.empty() && default_) { return *default_; /* most common case first */ } @@ -138,12 +138,12 @@ class Environment { initDefault(); } - if (!name) { + if (name.empty()) { ASSERT(default_); /* sanity check init was successful */ return *default_; } - std::map::iterator itr = communicators.find(name); + auto itr = communicators.find(name); if (itr != communicators.end()) { return *itr->second; } @@ -153,30 +153,30 @@ class Environment { for (itr = communicators.begin(); itr != communicators.end(); ++itr) { eckit::Log::error() << " " << (*itr).first << std::endl; } - throw eckit::SeriousBug(std::string("No communicator called ") + name, Here()); + throw eckit::SeriousBug("No communicator called " + std::string{name}, Here()); } - void addComm(const char* name, int comm) { + void addComm(std::string_view name, int comm) { AutoLock lock(mutex_); if (hasComm(name)) { - throw SeriousBug("Communicator with name " + std::string(name) + " already exists", Here()); + throw SeriousBug("Communicator with name " + std::string{name} + " already exists", Here()); } Comm* pComm = CommFactory::build(name, getDefaultCommType(), comm); - communicators[name] = pComm; + communicators.emplace(name, pComm); } - void addComm(const char* name, Comm* comm) { + void addComm(std::string_view name, Comm* comm) { AutoLock lock(mutex_); if (hasComm(name)) { - throw SeriousBug("Communicator with name " + std::string(name) + " already exists", Here()); + throw SeriousBug("Communicator with name " + std::string{name} + " already exists", Here()); } - communicators[name] = comm; + communicators.emplace(name, comm); } - void deleteComm(const char* name) { + void deleteComm(std::string_view name) { AutoLock lock(mutex_); auto itr = communicators.find(name); @@ -188,7 +188,7 @@ class Environment { // refuse to delete the default communicator if (default_ == comm) { throw SeriousBug("Trying to delete the default Communicator with name " - + std::string(name), + + std::string{name}, Here()); } @@ -198,7 +198,7 @@ class Environment { communicators.erase(itr); } else { - throw SeriousBug("Communicator with name " + std::string(name) + " does not exist", Here()); + throw SeriousBug("Communicator with name " + std::string{name} + " does not exist", Here()); } } @@ -215,7 +215,7 @@ class Environment { Comm* default_; - std::map communicators; + std::map> communicators; eckit::Mutex mutex_; }; @@ -224,21 +224,21 @@ class Environment { class CommFactories { public: - void registFactory(const std::string& builder, CommFactory* f) { + void registFactory(std::string_view builder, CommFactory* f) { AutoLock lock(mutex_); ASSERT(factories.find(builder) == factories.end()); - factories[builder] = f; + factories.emplace(builder, f); } - void unregistFactory(const std::string& builder) { + void unregistFactory(std::string_view builder) { AutoLock lock(mutex_); - factories.erase(builder); + factories.erase(std::string{builder}); } - CommFactory& getFactory(const std::string& builder) { + CommFactory& getFactory(std::string_view builder) const { AutoLock lock(mutex_); - std::map::const_iterator j = factories.find(builder); + auto j = factories.find(builder); if (j != factories.end()) { return *(j->second); @@ -250,7 +250,7 @@ class CommFactories { eckit::Log::error() << " " << (*j).first << std::endl; } - throw eckit::SeriousBug(std::string("No CommFactory called ") + builder, Here()); + throw eckit::SeriousBug("No CommFactory called " + std::string{builder}, Here()); } static CommFactories& instance() { @@ -261,11 +261,11 @@ class CommFactories { private: CommFactories() {} - std::map factories; - eckit::Mutex mutex_; + std::map> factories; + mutable eckit::Mutex mutex_; }; -CommFactory::CommFactory(const std::string& builder) : +CommFactory::CommFactory(std::string_view builder) : builder_(builder) { CommFactories::instance().registFactory(builder, this); } @@ -274,43 +274,43 @@ CommFactory::~CommFactory() { CommFactories::instance().unregistFactory(builder_); } -Comm* CommFactory::build(const std::string& name, const std::string& builder) { +Comm* CommFactory::build(std::string_view name, std::string_view builder) { return CommFactories::instance().getFactory(builder).make(name); } -Comm* CommFactory::build(const std::string& name, const std::string& builder, int comm) { +Comm* CommFactory::build(std::string_view name, std::string_view builder, int comm) { return CommFactories::instance().getFactory(builder).make(name, comm); } //---------------------------------------------------------------------------------------------------------------------- -Comm::Comm(const std::string& name) : +Comm::Comm(std::string_view name) : name_(name) {} Comm::~Comm() {} //---------------------------------------------------------------------------------------------------------------------- -Comm& comm(const char* name) { +Comm& comm(std::string_view name) { return Environment::instance().getComm(name); } -void setCommDefault(const char* name) { - Environment::instance().setDefaut(name); +void setCommDefault(std::string_view name) { + Environment::instance().setDefault(name); } -void addComm(const char* name, int comm) { +void addComm(std::string_view name, int comm) { Environment::instance().addComm(name, comm); } -void addComm(const char* name, Comm* comm) { +void addComm(std::string_view name, Comm* comm) { Environment::instance().addComm(name, comm); } -void deleteComm(const char* name) { +void deleteComm(std::string_view name) { Environment::instance().deleteComm(name); } -bool hasComm(const char* name) { +bool hasComm(std::string_view name) { return Environment::instance().hasComm(name); } diff --git a/src/eckit/mpi/Comm.h b/src/eckit/mpi/Comm.h index 5150d00c3..4c21635fe 100644 --- a/src/eckit/mpi/Comm.h +++ b/src/eckit/mpi/Comm.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include "eckit/filesystem/PathName.h" @@ -35,25 +36,25 @@ class Environment; /// @returns the communicator registered with associated name, or default communicator when NULL is /// passed -Comm& comm(const char* name = nullptr); +Comm& comm(std::string_view name = {}); Comm& self(); /// Set a communicator as default -void setCommDefault(const char* name); +void setCommDefault(std::string_view name); /// Register a communicator comming from Fortran code -void addComm(const char* name, int comm); +void addComm(std::string_view name, int comm); /// Register an existing communicator -void addComm(const char* name, Comm* comm); +void addComm(std::string_view name, Comm* comm); /// Unregister and delete specific comm /// @pre Comm is registered in the environment -void deleteComm(const char* name); +void deleteComm(std::string_view name); /// Check if a communicator is registered -bool hasComm(const char* name); +bool hasComm(std::string_view name); std::vector listComms(); @@ -78,7 +79,7 @@ class Comm : private eckit::NonCopyable { friend class Environment; public: // class methods - static Comm& comm(const char* name = nullptr); + static Comm& comm(std::string_view name = {}); public: // methods std::string name() const { return name_; } @@ -477,7 +478,7 @@ class Comm : private eckit::NonCopyable { } protected: // methods - Comm(const std::string& name); + Comm(std::string_view name); virtual ~Comm(); @@ -492,21 +493,21 @@ class CommFactory { friend class eckit::mpi::Environment; std::string builder_; - virtual Comm* make(const std::string& name) = 0; - virtual Comm* make(const std::string& name, int) = 0; + virtual Comm* make(std::string_view name) = 0; + virtual Comm* make(std::string_view name, int) = 0; protected: - CommFactory(const std::string& builder); + CommFactory(std::string_view builder); virtual ~CommFactory(); - static Comm* build(const std::string& name, const std::string& builder); - static Comm* build(const std::string& name, const std::string& builder, int); + static Comm* build(std::string_view name, std::string_view builder); + static Comm* build(std::string_view name, std::string_view builder, int); }; template class CommBuilder : public CommFactory { - Comm* make(const std::string& name) override { return new T(name); } - Comm* make(const std::string& name, int comm) override { return new T(name, comm); } + Comm* make(std::string_view name) override { return new T(name); } + Comm* make(std::string_view name, int comm) override { return new T(name, comm); } public: CommBuilder(const std::string& builder) : diff --git a/src/eckit/mpi/Parallel.cc b/src/eckit/mpi/Parallel.cc index 2479e4422..ea348fae9 100644 --- a/src/eckit/mpi/Parallel.cc +++ b/src/eckit/mpi/Parallel.cc @@ -55,7 +55,7 @@ class MPIError : public eckit::Exception { //---------------------------------------------------------------------------------------------------------------------- -void MPICall(int code, const char* mpifunc, const eckit::CodeLocation& loc) { +void MPICall(int code, std::string_view mpifunc, const eckit::CodeLocation& loc) { if (code != MPI_SUCCESS) { char error[10240]; @@ -169,7 +169,7 @@ size_t getSize(MPI_Comm comm) { //---------------------------------------------------------------------------------------------------------------------- -Parallel::Parallel(const std::string& name) : +Parallel::Parallel(std::string_view name) : Comm(name) /* don't use member initialisation list */ { pthread_once(&once, init); @@ -185,7 +185,7 @@ Parallel::Parallel(const std::string& name) : size_ = getSize(comm_); } -Parallel::Parallel(const std::string& name, MPI_Comm comm, bool) : +Parallel::Parallel(std::string_view name, MPI_Comm comm, bool) : Comm(name) /* don't use member initialisation list */ { pthread_once(&once, init); @@ -201,7 +201,7 @@ Parallel::Parallel(const std::string& name, MPI_Comm comm, bool) : size_ = getSize(comm_); } -Parallel::Parallel(const std::string& name, int comm) : +Parallel::Parallel(std::string_view name, int comm) : Comm(name) { pthread_once(&once, init); diff --git a/src/eckit/mpi/Parallel.h b/src/eckit/mpi/Parallel.h index f0abfae0e..0b8e99aae 100644 --- a/src/eckit/mpi/Parallel.h +++ b/src/eckit/mpi/Parallel.h @@ -29,9 +29,9 @@ class Parallel : public eckit::mpi::Comm { template friend class CommBuilder; - Parallel(const std::string& name); - Parallel(const std::string& name, MPI_Comm comm, bool); - Parallel(const std::string& name, int comm); + Parallel(std::string_view name); + Parallel(std::string_view name, MPI_Comm comm, bool); + Parallel(std::string_view name, int comm); ~Parallel() override; diff --git a/src/eckit/mpi/ParallelGroup.cc b/src/eckit/mpi/ParallelGroup.cc index 0d7feae24..4653ecbd3 100644 --- a/src/eckit/mpi/ParallelGroup.cc +++ b/src/eckit/mpi/ParallelGroup.cc @@ -8,6 +8,8 @@ * does it submit to any jurisdiction. */ +#include + #include "eckit/mpi/ParallelGroup.h" #include "eckit/log/CodeLocation.h" #include "eckit/mpi/Parallel.h" @@ -16,7 +18,7 @@ namespace eckit { namespace mpi { -void MPICall(int code, const char* mpifunc, const eckit::CodeLocation& loc); +void MPICall(int code, std::string_view mpifunc, const eckit::CodeLocation& loc); #define MPI_CALL(a) MPICall(a, #a, Here()) //---------------------------------------------------------------------------------------------------------------------- @@ -35,8 +37,7 @@ ParallelGroup::ParallelGroup(MPI_Group group) : group_(group) {} void ParallelGroup::print(std::ostream& os) const { - os << "ParallelGroup(" - << ")"; + os << "ParallelGroup()"; } int ParallelGroup::group() const { diff --git a/src/eckit/mpi/ParallelRequest.cc b/src/eckit/mpi/ParallelRequest.cc index 6710cd79e..67b1c0036 100644 --- a/src/eckit/mpi/ParallelRequest.cc +++ b/src/eckit/mpi/ParallelRequest.cc @@ -8,6 +8,8 @@ * does it submit to any jurisdiction. */ +#include + #include "eckit/mpi/ParallelRequest.h" #include "eckit/log/CodeLocation.h" #include "eckit/mpi/Parallel.h" @@ -16,7 +18,7 @@ namespace eckit { namespace mpi { -void MPICall(int code, const char* mpifunc, const eckit::CodeLocation& loc); +void MPICall(int code, std::string_view mpifunc, const eckit::CodeLocation& loc); #define MPI_CALL(a) MPICall(a, #a, Here()) //---------------------------------------------------------------------------------------------------------------------- diff --git a/src/eckit/mpi/Serial.cc b/src/eckit/mpi/Serial.cc index 923bf464a..70a49e1fd 100644 --- a/src/eckit/mpi/Serial.cc +++ b/src/eckit/mpi/Serial.cc @@ -134,13 +134,13 @@ class SerialRequestPool : private NonCopyable { //---------------------------------------------------------------------------------------------------------------------- -Serial::Serial(const std::string& name) : +Serial::Serial(std::string_view name) : Comm(name) { rank_ = 0; size_ = 1; } -Serial::Serial(const std::string& name, int) : +Serial::Serial(std::string_view name, int) : Comm(name) { rank_ = 0; size_ = 1; diff --git a/src/eckit/mpi/Serial.h b/src/eckit/mpi/Serial.h index 482fe60f2..b561b1531 100644 --- a/src/eckit/mpi/Serial.h +++ b/src/eckit/mpi/Serial.h @@ -30,8 +30,8 @@ class Serial : public eckit::mpi::Comm { template friend class CommBuilder; - Serial(const std::string& name); - Serial(const std::string& name, int); + Serial(std::string_view name); + Serial(std::string_view name, int); ~Serial() override; From 3a9f8f76ea899d37c9367d9c95b0df0a8f9addfa Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Mon, 6 May 2024 14:11:50 +0200 Subject: [PATCH 06/17] Add more verbosity to eckit_test_mpi to diagnose intermittent failures (#47) --- tests/mpi/eckit_test_mpi.cc | 39 ++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/tests/mpi/eckit_test_mpi.cc b/tests/mpi/eckit_test_mpi.cc index cb900e2d6..7469316ad 100644 --- a/tests/mpi/eckit_test_mpi.cc +++ b/tests/mpi/eckit_test_mpi.cc @@ -942,6 +942,9 @@ CASE("test_probe") { comm.barrier(); + std::stringstream errstr; + bool error = false; + auto count = nproc; while (count > 0) { auto status = comm.probe(comm.anySource(), comm.anyTag()); @@ -952,11 +955,20 @@ CASE("test_probe") { EXPECT(sz == 1); comm.receive(&data[status.source()], sz, status.source(), status.tag()); + errstr << "[test_probe:" << irank << "] receive data["< sts = comm.waitAll(requests); @@ -964,6 +976,9 @@ CASE("test_probe") { for (auto& st : sts) { EXPECT(st.error() == 0); } + + comm.barrier(); + EXPECT(!error); } CASE("test_iProbe") { @@ -982,6 +997,9 @@ CASE("test_iProbe") { comm.barrier(); + std::stringstream errstr; + bool error = false; + auto count = nproc; while (count > 0) { auto status = comm.iProbe(comm.anySource(), comm.anyTag()); @@ -996,12 +1014,20 @@ CASE("test_iProbe") { EXPECT(sz == 1); comm.receive(&data[status.source()], sz, status.source(), status.tag()); - + errstr << "[test_iProbe:" << irank << "] receive data["< sts = comm.waitAll(requests); @@ -1009,6 +1035,9 @@ CASE("test_iProbe") { for (auto& st : sts) { EXPECT(st.error() == 0); } + + comm.barrier(); + EXPECT(!error); } //---------------------------------------------------------------------------------------------------------------------- From 522368258f9b5c8d9b09d5754a46904e4519fe78 Mon Sep 17 00:00:00 2001 From: Nicolau Manubens Date: Fri, 17 May 2024 00:39:35 +0100 Subject: [PATCH 07/17] Avoids repeatedly building regexes in Time construction from string. --- src/eckit/types/Time.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/eckit/types/Time.cc b/src/eckit/types/Time.cc index f10eb9ba8..402253f22 100644 --- a/src/eckit/types/Time.cc +++ b/src/eckit/types/Time.cc @@ -18,6 +18,13 @@ #include "eckit/utils/Hash.h" #include "eckit/utils/Tokenizer.h" +namespace { + static thread_local std::regex digits_time_("^-?[0-9]+$"); + static thread_local std::regex float_hours_("^-?[0-9]*\\.[0-9]+$"); + static thread_local std::regex hhmmss_("^([0-9]+):([0-5]?[0-9])(:[0-5]?[0-9])?$"); + static thread_local std::regex ddhhmmss_("^-?([0-9]+[dD])?([0-9]+[hH])?([0-9]+[mM])?([0-9]+[sS])?$"); +} + namespace eckit { //---------------------------------------------------------------------------------------------------------------------- @@ -45,7 +52,7 @@ Time::Time(const std::string& s, bool extended) { long dd = 0; std::smatch m; - if (std::regex_match (s, m, std::regex("^-?[0-9]+$"))) { // only digits + if (std::regex_match (s, m, digits_time_)) { long t = std::stol(s); int sign = (s[0] == '-' ? 1 : 0); if (extended || s.length() <= 2+sign) { // cases: h, hh, (or hhh..h for step parsing) @@ -62,7 +69,7 @@ Time::Time(const std::string& s, bool extended) { } } else { - if (std::regex_match (s, m, std::regex("^-?[0-9]*\\.[0-9]+$"))) { // floating point (hours) + if (std::regex_match (s, m, float_hours_)) { long sec = std::round(std::stod(s)*3600); hh = sec/3600; sec -= hh*3600; @@ -71,7 +78,7 @@ Time::Time(const std::string& s, bool extended) { ss = sec; } else { - if (std::regex_match (s, m, std::regex("^([0-9]+):([0-5]?[0-9])(:[0-5]?[0-9])?$"))) { + if (std::regex_match (s, m, hhmmss_)) { for (int i=1; i Date: Tue, 21 May 2024 10:42:05 +0200 Subject: [PATCH 08/17] Fix uint64_t cast in TimeStamp.cc --- src/eckit/log/TimeStamp.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/eckit/log/TimeStamp.cc b/src/eckit/log/TimeStamp.cc index 0a2d36d20..be5a5a41d 100644 --- a/src/eckit/log/TimeStamp.cc +++ b/src/eckit/log/TimeStamp.cc @@ -10,6 +10,7 @@ #include #include +#include #include "eckit/eckit.h" @@ -30,7 +31,7 @@ TimeStamp::TimeStamp(time_t t, const std::string& format) : std::ostream& operator<<(std::ostream& s, const TimeStamp& x) { if (x.format_ == "hex") { - s << std::setw(16) << std::setfill('0') << std::hex << (uint64_t) x.time_; + s << std::setw(16) << std::setfill('0') << std::hex << static_cast(x.time_); return s; } From 4f319ac3dc9205bfa25847de11e405790d7d99f8 Mon Sep 17 00:00:00 2001 From: sametd Date: Mon, 27 May 2024 21:09:15 +0000 Subject: [PATCH 09/17] basic implementation for structured data (only for origin) --- src/eckit/log/SysLog.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/eckit/log/SysLog.h b/src/eckit/log/SysLog.h index 28931e295..c7738b9dc 100644 --- a/src/eckit/log/SysLog.h +++ b/src/eckit/log/SysLog.h @@ -99,6 +99,11 @@ class SysLog { return s; } + /// Optional fields for structured data (RFC 5424 section 6.3) + void software(const std::string& software) { software_ = software; } + void swVersion(const std::string& version) { swVersion_ = version; } + void enterpriseId(const std::string& id) { enterpriseId_ = id; } + private: // methods void print(std::ostream& out) const; @@ -112,6 +117,11 @@ class SysLog { int msgid_; std::string msg_; + + // optional fields for structured data + std::string software_; + std::string swVersion_; + std::string enterpriseId_; }; } // namespace eckit From 3662286b104cee407c4ba4c16eca986f0a14bd03 Mon Sep 17 00:00:00 2001 From: sametd Date: Mon, 27 May 2024 21:32:26 +0000 Subject: [PATCH 10/17] initial structured data implementation for syslog (only origin part) --- src/eckit/log/SysLog.cc | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/eckit/log/SysLog.cc b/src/eckit/log/SysLog.cc index 98c5ffc54..02f9e783b 100644 --- a/src/eckit/log/SysLog.cc +++ b/src/eckit/log/SysLog.cc @@ -18,12 +18,13 @@ #include "eckit/runtime/Main.h" #include "eckit/log/TimeStamp.h" +#include "eckit/net/IPAddress.h" namespace eckit { -SysLog::SysLog(const std::string& msg, int msgid, Facility f, Severity s) : - facility_(f), severity_(s), appName_(Main::instance().name()), msgid_(msgid), msg_(msg) { +SysLog::SysLog(const std::string& msg, int msgid, Facility f, Severity s) + : facility_(f), severity_(s), appName_(Main::instance().name()), msgid_(msgid), msg_(msg) { timestamp_ = TimeStamp("%Y-%m-%dT%H:%M:%SZ"); ///< assumes we are in UTC } @@ -43,11 +44,26 @@ int SysLog::procid() const { return ::getpid(); } + std::string SysLog::structuredData() const { + if (software_.empty() && swVersion_.empty() && enterpriseId_.empty()) { + return std::string(1, nilvalue()); + } + // RFC 5424 section 6.3 (only origin) std::ostringstream s; - - /// @todo Implement the structured message meta-description as described in RFC5424 secion 6.3 - s << nilvalue(); + std::string ip = net::IPAddress::myIPAddress().asString(); + + s << "[origin ip=\"" << ip << "\""; + if (!enterpriseId_.empty()) { + s << " enterpriseId=\"" << enterpriseId_ << "\""; + } + if (!software_.empty()) { + s << " software=\"" << software_ << "\""; + if (!swVersion_.empty()) { + s << " swVersion=\"" << swVersion_ << "\""; + } + } + s << "]"; return s.str(); } From 7e4ee5dfd7ec7a6f2a610fe4f966f71da7e82b45 Mon Sep 17 00:00:00 2001 From: sametd Date: Mon, 27 May 2024 21:33:19 +0000 Subject: [PATCH 11/17] clang-format --- src/eckit/log/SysLog.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/eckit/log/SysLog.cc b/src/eckit/log/SysLog.cc index 02f9e783b..ab7371720 100644 --- a/src/eckit/log/SysLog.cc +++ b/src/eckit/log/SysLog.cc @@ -23,8 +23,8 @@ namespace eckit { -SysLog::SysLog(const std::string& msg, int msgid, Facility f, Severity s) - : facility_(f), severity_(s), appName_(Main::instance().name()), msgid_(msgid), msg_(msg) { +SysLog::SysLog(const std::string& msg, int msgid, Facility f, Severity s) : + facility_(f), severity_(s), appName_(Main::instance().name()), msgid_(msgid), msg_(msg) { timestamp_ = TimeStamp("%Y-%m-%dT%H:%M:%SZ"); ///< assumes we are in UTC } From 8b616794f5f23fced3a66002fb454d21af504408 Mon Sep 17 00:00:00 2001 From: sametd Date: Mon, 27 May 2024 21:34:18 +0000 Subject: [PATCH 12/17] spelling correction --- src/eckit/log/SysLog.cc | 2 +- src/eckit/log/SysLog.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/eckit/log/SysLog.cc b/src/eckit/log/SysLog.cc index ab7371720..e10304ad3 100644 --- a/src/eckit/log/SysLog.cc +++ b/src/eckit/log/SysLog.cc @@ -75,7 +75,7 @@ SysLog::operator std::string() const { static char sep = ' '; os // RFC 5424 section 6.2 (Header) - << "<" << priotity() << ">" << version() << sep << timestamp() << sep << fqdn() << sep << appName() << sep + << "<" << priority() << ">" << version() << sep << timestamp() << sep << fqdn() << sep << appName() << sep << procid() << sep << msgid() << sep // RFC 5424 section 6.3 diff --git a/src/eckit/log/SysLog.h b/src/eckit/log/SysLog.h index c7738b9dc..d88c31f53 100644 --- a/src/eckit/log/SysLog.h +++ b/src/eckit/log/SysLog.h @@ -72,7 +72,7 @@ class SysLog { SysLog(const std::string& msg, int msgid = 0, Facility f = SysLog::User, Severity s = SysLog::Info); - unsigned priotity() const { return facility_ * 8 + severity_; } + unsigned priority() const { return facility_ * 8 + severity_; } unsigned version() const { return 1; } From 65981d628ac8d532d04d2e197a2c16531834521e Mon Sep 17 00:00:00 2001 From: sametd Date: Tue, 28 May 2024 13:35:59 +0000 Subject: [PATCH 13/17] added tests for syslog class --- tests/log/test_syslog.cc | 84 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 tests/log/test_syslog.cc diff --git a/tests/log/test_syslog.cc b/tests/log/test_syslog.cc new file mode 100644 index 000000000..ab602a23a --- /dev/null +++ b/tests/log/test_syslog.cc @@ -0,0 +1,84 @@ +/* + * (C) Copyright 1996- ECMWF. + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + * In applying this licence, ECMWF does not waive the privileges and immunities + * granted to it by virtue of its status as an intergovernmental organisation nor + * does it submit to any jurisdiction. + */ + +#include + +#include "eckit/log/SysLog.h" +#include "eckit/testing/Test.h" + +using namespace std; +using namespace eckit; +using namespace eckit::testing; + +namespace eckit::test { + +//---------------------------------------------------------------------------------------------------------------------- + +CASE("test_priority") { + SysLog log("Test message", 0, SysLog::Local7, SysLog::Info); + std::string logString = static_cast(log); + std::string expectedPriority = "<" + std::to_string(log.priority()) + ">"; + EXPECT(logString.find(expectedPriority) != std::string::npos); +} + +//---------------------------------------------------------------------------------------------------------------------- + +CASE("test_timezone") { + SysLog log("Test message", 0, SysLog::Local7, SysLog::Info); + std::string logString = static_cast(log); + // Check if 'Z' UTC indicator is present + EXPECT(logString.find("Z") != std::string::npos); + // Check if 'T' separator is present + EXPECT(logString.find("T") != std::string::npos); +} + +//---------------------------------------------------------------------------------------------------------------------- + +CASE("test_appname") { + SysLog log("Test message", 0, SysLog::Local7, SysLog::Info); + EXPECT(log.appName() == Main::instance().name()); + + // Change the appName and check if it persists + log.appName("test_app"); + std::string logString = static_cast(log); + EXPECT(logString.find("test_app") != std::string::npos); + + // Create a new SysLog instance and check if it retains the original appName + SysLog newLog("New message", 2, SysLog::Local7, SysLog::Info); + EXPECT(newLog.appName() == Main::instance().name()); +} + +//---------------------------------------------------------------------------------------------------------------------- + +CASE("test_without_structured_data") { + SysLog log("Test message", 0, SysLog::Local7, SysLog::Info); + std::string logString = static_cast(log); + // Check if structured data is not present + EXPECT(logString.find("[origin") == std::string::npos); +} + +//---------------------------------------------------------------------------------------------------------------------- +CASE("test_with_structured_data") { + SysLog log("Test message", 0, SysLog::Local7, SysLog::Info); + log.software("log_test"); + log.swVersion("1.0.0"); + log.enterpriseId("7464"); + std::string logString = static_cast(log); + EXPECT(logString.find("enterpriseId=\"7464\"") != std::string::npos); + EXPECT(logString.find("software=\"log_test\"") != std::string::npos); + EXPECT(logString.find("swVersion=\"1.0.0\"") != std::string::npos); +} +//---------------------------------------------------------------------------------------------------------------------- + +} // namespace eckit::test + +int main(int argc, char** argv) { + return run_tests(argc, argv); +} From 52ba68964e71f4598e90af50d0ba545d2f4ad77a Mon Sep 17 00:00:00 2001 From: sametd Date: Tue, 28 May 2024 13:36:13 +0000 Subject: [PATCH 14/17] added syslog test --- tests/log/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/log/CMakeLists.txt b/tests/log/CMakeLists.txt index fbd95d313..58f9346bc 100644 --- a/tests/log/CMakeLists.txt +++ b/tests/log/CMakeLists.txt @@ -27,3 +27,7 @@ ecbuild_add_test( TARGET eckit_test_log_user_channels ENABLED OFF SOURCES test_log_user_channels.cc LIBS eckit ) + +ecbuild_add_test( TARGET eckit_test_syslog + SOURCES test_syslog.cc + LIBS eckit ) \ No newline at end of file From cd117d37a1815454fafa444b6f743f4b44a47443 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Tue, 9 Jan 2024 16:27:51 +0100 Subject: [PATCH 15/17] Add introspection to eckit::Configuration --- src/eckit/config/Configuration.cc | 101 +++++++++++++++++++++++++++++ src/eckit/config/Configuration.h | 80 +++++++++++++++++++++++ tests/config/test_configuration.cc | 44 +++++++++++++ 3 files changed, 225 insertions(+) diff --git a/src/eckit/config/Configuration.cc b/src/eckit/config/Configuration.cc index acc93bc1e..b858cc560 100644 --- a/src/eckit/config/Configuration.cc +++ b/src/eckit/config/Configuration.cc @@ -460,6 +460,107 @@ LocalConfiguration Configuration::getSubConfiguration(const std::string& name) c return result; } +bool Configuration::isIntegral(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + return found && v.isNumber(); +} + +bool Configuration::isBoolean(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + return found && v.isBool(); +} + +bool Configuration::isFloatingPoint(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + return found && v.isDouble(); +} + +bool Configuration::isString(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + return found && v.isString(); +} + +bool Configuration::isList(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + return found && v.isList(); +} + +bool Configuration::isSubConfiguration(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + return found && (v.isMap() || v.isOrderedMap()); +} + +bool Configuration::isIntegralList(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + if (found && v.isList()) { + if (v.size() == 0) { + return true; + } + auto& firstElement = v[0]; + return firstElement.isNumber(); + } + return false; +} + +bool Configuration::isBooleanList(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + if (found && v.isList()) { + if (v.size() == 0) { + return true; + } + auto& firstElement = v[0]; + return firstElement.isBool(); + } + return false; +} + +bool Configuration::isFloatingPointList(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + if (found && v.isList()) { + if (v.size() == 0) { + return true; + } + auto& firstElement = v[0]; + return firstElement.isDouble(); + } + return false; +} + +bool Configuration::isStringList(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + if (found && v.isList()) { + if (v.size() == 0) { + return true; + } + auto& firstElement = v[0]; + return firstElement.isString(); + } + return false; +} + +bool Configuration::isSubConfigurationList(const std::string& name) const { + bool found = false; + eckit::Value v = lookUp(name, found); + if (found && v.isList()) { + if (v.size() == 0) { + return true; + } + auto& firstElement = v[0]; + return firstElement.isMap() || firstElement.isOrderedMap(); + } + return false; +} + template void Configuration::_getWithDefault(const std::string& name, T& value, const T& defaultVal) const { if (!get(name, value)) { diff --git a/src/eckit/config/Configuration.h b/src/eckit/config/Configuration.h index 8adfacc2e..6c795ed11 100644 --- a/src/eckit/config/Configuration.h +++ b/src/eckit/config/Configuration.h @@ -18,6 +18,7 @@ #include #include +#include #include "eckit/config/Parametrisation.h" @@ -133,6 +134,70 @@ class Configuration : public Parametrisation { virtual void hash(eckit::Hash&) const; + // -- Introspection methods + + bool isSubConfiguration(const std::string& name) const; + + bool isIntegral(const std::string& name) const; + + bool isBoolean(const std::string& name) const; + + bool isFloatingPoint(const std::string& name) const; + + bool isString(const std::string& name) const; + + bool isList(const std::string& name) const; + + bool isSubConfigurationList(const std::string& name) const; + + bool isIntegralList(const std::string& name) const; + + bool isBooleanList(const std::string& name) const; + + bool isFloatingPointList(const std::string& name) const; + + bool isStringList(const std::string& name) const; + + template + bool isConvertible(const std::string& name) const { + using _T = std::decay_t; + if constexpr(std::is_base_of_v) { + return isSubConfiguration(name); + } + else if constexpr(std::is_same_v<_T,int> || std::is_same_v<_T,long> || std::is_same_v<_T,long long> || std::is_same_v<_T,std::size_t>) { + return isIntegral(name) || isBoolean(name); + } + else if constexpr(std::is_same_v<_T,float> || std::is_same_v<_T,double>) { + return isFloatingPoint(name) || isIntegral(name) || isBoolean(name); + } + else if constexpr(std::is_same_v<_T,std::string>) { + return isString(name); + } + else if constexpr(is_vector<_T>::value) { + using _V = std::decay_t; + if constexpr(std::is_base_of_v) { + return isSubConfigurationList(name); + } + else if constexpr(std::is_same_v<_V,int> || std::is_same_v<_V,long> || std::is_same_v<_V,long long> || std::is_same_v<_V,std::size_t>) { + return isIntegralList(name) || isBooleanList(name); + } + else if constexpr(std::is_same_v<_V,float> || std::is_same_v<_V,double>) { + return isFloatingPointList(name) || isIntegralList(name) || isBooleanList(name); + } + else if constexpr(std::is_same_v<_V,std::string>) { + return isStringList(name); + } + } + else { + return false; + } + } + + template + bool isConvertible(const std::string& name, T&) const { + return isConvertible(name); + } + protected: // methods Configuration(const eckit::Value&, char separator = '.'); @@ -173,6 +238,21 @@ class Configuration : public Parametrisation { p.print(s); return s; } + +private: + + // Helper structs for introspection of template T in isConvertible method + template + struct is_vector { + using type = T ; + constexpr static bool value = false; + }; + + template + struct is_vector> { + using type = std::vector ; + constexpr static bool value = true; + }; }; //---------------------------------------------------------------------------------------------------------------------- diff --git a/tests/config/test_configuration.cc b/tests/config/test_configuration.cc index 623ed2646..624cec6a3 100644 --- a/tests/config/test_configuration.cc +++ b/tests/config/test_configuration.cc @@ -353,6 +353,8 @@ CASE("test_local_configuration") { LocalConfiguration manager; manager.set("name", "Sidonia"); manager.set("office", 1); + manager.set("height", 1.78); + manager.set("free", false); std::vector staff(2); staff[0].set("name", "Suske"); @@ -362,6 +364,8 @@ CASE("test_local_configuration") { local.set("manager", manager); local.set("staff", staff); + + local.set("books.count", 10); } const Configuration& conf = local; @@ -369,6 +373,9 @@ CASE("test_local_configuration") { std::vector staff; EXPECT(conf.get("manager", manager)); + + EXPECT(conf.isSubConfigurationList("staff")); + EXPECT(conf.isConvertible("staff",staff)); EXPECT(conf.get("staff", staff)); std::string name; @@ -388,6 +395,43 @@ CASE("test_local_configuration") { EXPECT(staff[1].get("office", office)); EXPECT(name == std::string("Wiske")); EXPECT(office == 3); + + int books_count; + EXPECT(conf.has("books")); + EXPECT(conf.get("books.count", books_count)); + EXPECT(books_count == 10); + + LocalConfiguration books; + EXPECT(conf.isSubConfiguration("books")); + conf.get("books",books); + EXPECT(books.getInt("count") == 10); + + EXPECT(conf.isConvertible("books")); + + EXPECT(conf.isList("staff")); + EXPECT(conf.isIntegral("manager.office")); + EXPECT(!conf.isFloatingPoint("manager.office")); + EXPECT(conf.isConvertible("manager.office")); + EXPECT(conf.isConvertible("manager.office")); + EXPECT(conf.isConvertible("manager.office")); + EXPECT(conf.isConvertible("manager.office")); + EXPECT(conf.isConvertible("manager.office")); + EXPECT(conf.isConvertible("manager.office")); + EXPECT(!conf.isConvertible("manager.office")); + EXPECT(!conf.isConvertible("manager.office")); + EXPECT(!conf.isConvertible("manager.office")); + + EXPECT(conf.isConvertible("manager.height")); + EXPECT(conf.isConvertible("manager.height")); + EXPECT(!conf.isConvertible("manager.height")); + EXPECT(!conf.isConvertible("manager.height")); + EXPECT(!conf.isConvertible("manager.height")); + EXPECT(!conf.isConvertible("manager.height")); + + double manager_height; + EXPECT(conf.get("manager.height", manager_height)); + EXPECT(manager_height == 1.78); + } CASE("Hash a configuration") { From 44e7ddd124858a3acf34a2f73d0fd3a317842af8 Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Fri, 12 Jan 2024 14:36:41 +0100 Subject: [PATCH 16/17] More robust Configuration::has with nested configuration --- src/eckit/config/Configuration.cc | 4 ++++ tests/config/test_configuration.cc | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/eckit/config/Configuration.cc b/src/eckit/config/Configuration.cc index b858cc560..737106c6e 100644 --- a/src/eckit/config/Configuration.cc +++ b/src/eckit/config/Configuration.cc @@ -73,6 +73,10 @@ eckit::Value Configuration::lookUp(const std::string& s, bool& found) const { eckit::Value result = *root_; for (size_t i = 0; i < path.size(); i++) { + if (!(result.isMap() || result.isOrderedMap())) { + found = false; + return result; + } const std::string& key = path[i]; if (!result.contains(key)) { found = false; diff --git a/tests/config/test_configuration.cc b/tests/config/test_configuration.cc index 624cec6a3..e303f5426 100644 --- a/tests/config/test_configuration.cc +++ b/tests/config/test_configuration.cc @@ -432,6 +432,10 @@ CASE("test_local_configuration") { EXPECT(conf.get("manager.height", manager_height)); EXPECT(manager_height == 1.78); + local.set("a", "a"); + const eckit::Parametrisation& p = conf; + EXPECT(!p.has("a.b")); + } CASE("Hash a configuration") { From c2d6450ff328ba42c130f382594b7d7d29ac22fc Mon Sep 17 00:00:00 2001 From: Emanuele Danovaro Date: Tue, 4 Jun 2024 15:10:49 +0200 Subject: [PATCH 17/17] version bump --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index c7c3f3333..f8f738140 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.26.2 +1.26.3