From 505e68e60103f7fd2723a99cab1a11557c6fa679 Mon Sep 17 00:00:00 2001 From: Giovanni De Toni Date: Thu, 18 Apr 2019 15:49:42 +0200 Subject: [PATCH 1/4] Prepend a C in front of concrete ParameterObserver classes. It is more consistent with Shogun's code style and it make them accessible from the SWIG interfaces (factories). --- .../lib/observers/ParameterObserverHistogram.cpp | 14 +++++++------- .../lib/observers/ParameterObserverHistogram.h | 10 +++++----- .../lib/observers/ParameterObserverLogger.cpp | 12 ++++++------ src/shogun/lib/observers/ParameterObserverLogger.h | 8 ++++---- .../lib/observers/ParameterObserverScalar.cpp | 14 +++++++------- src/shogun/lib/observers/ParameterObserverScalar.h | 10 +++++----- .../lib/observers/ParameterObserverTensorBoard.cpp | 2 +- tests/unit/base/SGObject_unittest.cc | 8 ++++---- .../observers/ParameterObserverScalar_unittest.cc | 6 +++--- 9 files changed, 42 insertions(+), 42 deletions(-) diff --git a/src/shogun/lib/observers/ParameterObserverHistogram.cpp b/src/shogun/lib/observers/ParameterObserverHistogram.cpp index b89e8452c67..41552becbd9 100644 --- a/src/shogun/lib/observers/ParameterObserverHistogram.cpp +++ b/src/shogun/lib/observers/ParameterObserverHistogram.cpp @@ -40,28 +40,28 @@ using namespace shogun; -ParameterObserverHistogram::ParameterObserverHistogram() +CParameterObserverHistogram::CParameterObserverHistogram() : ParameterObserverTensorBoard() { } -ParameterObserverHistogram::ParameterObserverHistogram( +CParameterObserverHistogram::CParameterObserverHistogram( std::vector& parameters) : ParameterObserverTensorBoard(parameters) { } -ParameterObserverHistogram::ParameterObserverHistogram( +CParameterObserverHistogram::CParameterObserverHistogram( const std::string& filename, std::vector& parameters) : ParameterObserverTensorBoard(filename, parameters) { } -ParameterObserverHistogram::~ParameterObserverHistogram() +CParameterObserverHistogram::~CParameterObserverHistogram() { } -void ParameterObserverHistogram::on_next_impl(const TimedObservedValue& value) +void CParameterObserverHistogram::on_next_impl(const TimedObservedValue& value) { auto node_name = std::string("node"); auto format = TBOutputFormat(); @@ -69,11 +69,11 @@ void ParameterObserverHistogram::on_next_impl(const TimedObservedValue& value) m_writer.writeEvent(event_value); } -void ParameterObserverHistogram::on_error(std::exception_ptr) +void CParameterObserverHistogram::on_error(std::exception_ptr) { } -void ParameterObserverHistogram::on_complete() +void CParameterObserverHistogram::on_complete() { } diff --git a/src/shogun/lib/observers/ParameterObserverHistogram.h b/src/shogun/lib/observers/ParameterObserverHistogram.h index a8213ea57a9..476cfd94522 100644 --- a/src/shogun/lib/observers/ParameterObserverHistogram.h +++ b/src/shogun/lib/observers/ParameterObserverHistogram.h @@ -47,15 +47,15 @@ namespace shogun * Implementation of a ParameterObserver which write to file * histograms, given object emitted from a parameter observable. */ - class ParameterObserverHistogram : public ParameterObserverTensorBoard + class CParameterObserverHistogram : public ParameterObserverTensorBoard { public: - ParameterObserverHistogram(); - ParameterObserverHistogram(std::vector& parameters); - ParameterObserverHistogram( + CParameterObserverHistogram(); + CParameterObserverHistogram(std::vector& parameters); + CParameterObserverHistogram( const std::string& filename, std::vector& parameters); - ~ParameterObserverHistogram(); + ~CParameterObserverHistogram(); virtual void on_error(std::exception_ptr); virtual void on_complete(); diff --git a/src/shogun/lib/observers/ParameterObserverLogger.cpp b/src/shogun/lib/observers/ParameterObserverLogger.cpp index 6fc03971ac4..6a6f944db32 100644 --- a/src/shogun/lib/observers/ParameterObserverLogger.cpp +++ b/src/shogun/lib/observers/ParameterObserverLogger.cpp @@ -10,24 +10,24 @@ using namespace shogun; -ParameterObserverLogger::ParameterObserverLogger() {} +CParameterObserverLogger::CParameterObserverLogger() {} -ParameterObserverLogger::ParameterObserverLogger(std::vector ¶meters) : ParameterObserver( +CParameterObserverLogger::CParameterObserverLogger(std::vector ¶meters) : ParameterObserver( parameters) {} -ParameterObserverLogger::~ParameterObserverLogger() { +CParameterObserverLogger::~CParameterObserverLogger() { } -void ParameterObserverLogger::on_error(std::exception_ptr ptr) { +void CParameterObserverLogger::on_error(std::exception_ptr ptr) { } -void ParameterObserverLogger::on_complete() { +void CParameterObserverLogger::on_complete() { } -void ParameterObserverLogger::on_next_impl(const TimedObservedValue &value) { +void CParameterObserverLogger::on_next_impl(const TimedObservedValue &value) { auto name = value.first->get("name"); auto any_val = value.first->get_any(); diff --git a/src/shogun/lib/observers/ParameterObserverLogger.h b/src/shogun/lib/observers/ParameterObserverLogger.h index 263c346b9bb..b70740c763b 100644 --- a/src/shogun/lib/observers/ParameterObserverLogger.h +++ b/src/shogun/lib/observers/ParameterObserverLogger.h @@ -14,14 +14,14 @@ namespace shogun { /** * This class implements a logger which prints all observed updates. */ - class ParameterObserverLogger : public ParameterObserver { + class CParameterObserverLogger : public ParameterObserver { public: - ParameterObserverLogger(); + CParameterObserverLogger(); - ParameterObserverLogger(std::vector ¶meters); + CParameterObserverLogger(std::vector ¶meters); - virtual ~ParameterObserverLogger(); + virtual ~CParameterObserverLogger(); virtual void on_error(std::exception_ptr ptr); diff --git a/src/shogun/lib/observers/ParameterObserverScalar.cpp b/src/shogun/lib/observers/ParameterObserverScalar.cpp index c0c86eb7582..9d171c63071 100644 --- a/src/shogun/lib/observers/ParameterObserverScalar.cpp +++ b/src/shogun/lib/observers/ParameterObserverScalar.cpp @@ -40,28 +40,28 @@ using namespace shogun; -ParameterObserverScalar::ParameterObserverScalar() +CParameterObserverScalar::CParameterObserverScalar() : ParameterObserverTensorBoard() { } -ParameterObserverScalar::ParameterObserverScalar( +CParameterObserverScalar::CParameterObserverScalar( std::vector& parameters) : ParameterObserverTensorBoard(parameters) { } -ParameterObserverScalar::ParameterObserverScalar( +CParameterObserverScalar::CParameterObserverScalar( const std::string& filename, std::vector& parameters) : ParameterObserverTensorBoard(filename, parameters) { } -ParameterObserverScalar::~ParameterObserverScalar() +CParameterObserverScalar::~CParameterObserverScalar() { } -void ParameterObserverScalar::on_next_impl(const TimedObservedValue& value) +void CParameterObserverScalar::on_next_impl(const TimedObservedValue& value) { auto node_name = std::string("node"); auto format = TBOutputFormat(); @@ -69,11 +69,11 @@ void ParameterObserverScalar::on_next_impl(const TimedObservedValue& value) m_writer.writeEvent(event_value); } -void ParameterObserverScalar::on_error(std::exception_ptr) +void CParameterObserverScalar::on_error(std::exception_ptr) { } -void ParameterObserverScalar::on_complete() +void CParameterObserverScalar::on_complete() { } diff --git a/src/shogun/lib/observers/ParameterObserverScalar.h b/src/shogun/lib/observers/ParameterObserverScalar.h index 70908755539..d56cb232670 100644 --- a/src/shogun/lib/observers/ParameterObserverScalar.h +++ b/src/shogun/lib/observers/ParameterObserverScalar.h @@ -47,15 +47,15 @@ namespace shogun * Implementation of a ParameterObserver which write to file * scalar values, given object emitted from a parameter observable. */ - class ParameterObserverScalar : public ParameterObserverTensorBoard + class CParameterObserverScalar : public ParameterObserverTensorBoard { public: - ParameterObserverScalar(); - ParameterObserverScalar(std::vector& parameters); - ParameterObserverScalar( + CParameterObserverScalar(); + CParameterObserverScalar(std::vector& parameters); + CParameterObserverScalar( const std::string& filename, std::vector& parameters); - ~ParameterObserverScalar(); + ~CParameterObserverScalar(); virtual void on_error(std::exception_ptr); virtual void on_complete(); diff --git a/src/shogun/lib/observers/ParameterObserverTensorBoard.cpp b/src/shogun/lib/observers/ParameterObserverTensorBoard.cpp index f54bbb1f3da..f996d4adc83 100644 --- a/src/shogun/lib/observers/ParameterObserverTensorBoard.cpp +++ b/src/shogun/lib/observers/ParameterObserverTensorBoard.cpp @@ -35,7 +35,7 @@ #include #ifdef HAVE_TFLOGGER -#include "ParameterObserverTensorBoard.h" +#include using namespace shogun; diff --git a/tests/unit/base/SGObject_unittest.cc b/tests/unit/base/SGObject_unittest.cc index 664685ff527..ac80bf87ea4 100644 --- a/tests/unit/base/SGObject_unittest.cc +++ b/tests/unit/base/SGObject_unittest.cc @@ -573,7 +573,7 @@ TEST(SGObject, watch_method) TEST(SGObject, subscribe_observer) { auto obj = some(); - auto param_obs = some(); + auto param_obs = some(); obj->subscribe(param_obs); EXPECT_EQ(param_obs->get("subscription_id"), 0); @@ -583,7 +583,7 @@ TEST(SGObject, subscribe_observer) TEST(SGObject, unsubscribe_observer) { auto obj = some(); - auto param_obs = some(); + auto param_obs = some(); obj->subscribe(param_obs); obj->unsubscribe(param_obs); @@ -594,8 +594,8 @@ TEST(SGObject, unsubscribe_observer) TEST(SGObject, unsubscribe_observer_failure) { auto obj = some(); - auto param_obs = some(); - auto param_obs_not_in = some(); + auto param_obs = some(); + auto param_obs_not_in = some(); EXPECT_THROW(obj->unsubscribe(param_obs_not_in), ShogunException); } diff --git a/tests/unit/lib/observers/ParameterObserverScalar_unittest.cc b/tests/unit/lib/observers/ParameterObserverScalar_unittest.cc index f2070afc1b6..01d95d77413 100644 --- a/tests/unit/lib/observers/ParameterObserverScalar_unittest.cc +++ b/tests/unit/lib/observers/ParameterObserverScalar_unittest.cc @@ -46,13 +46,13 @@ using namespace shogun; TEST(ParameterObserverScalar, filter_empty) { - ParameterObserverScalar tmp; + CParameterObserverScalar tmp; EXPECT_TRUE(tmp.filter("a")); } TEST(ParameterObserverScalar, filter_found) { - ParameterObserverScalar tmp{test_params}; + CParameterObserverScalar tmp{test_params}; EXPECT_TRUE(tmp.filter("a")); EXPECT_TRUE(tmp.filter("b")); EXPECT_TRUE(tmp.filter("c")); @@ -61,7 +61,7 @@ TEST(ParameterObserverScalar, filter_found) TEST(ParameterObserverScalar, filter_not_found) { - ParameterObserverScalar tmp{test_params}; + CParameterObserverScalar tmp{test_params}; EXPECT_FALSE(tmp.filter("k")); } From 0a7b45a0bbad044b1305ddcdb63266c471f41919 Mon Sep 17 00:00:00 2001 From: Giovanni De Toni Date: Thu, 18 Apr 2019 16:26:41 +0200 Subject: [PATCH 2/4] Fix some bugs inside ParameterObserverLogger. * Change %l to %lu (it was causing a segfault inside SG_PRINT); * Add a try block to catch exceptions from sg_any_dispatch; --- .../lib/observers/ParameterObserverLogger.cpp | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/shogun/lib/observers/ParameterObserverLogger.cpp b/src/shogun/lib/observers/ParameterObserverLogger.cpp index 6a6f944db32..73e3685d8b8 100644 --- a/src/shogun/lib/observers/ParameterObserverLogger.cpp +++ b/src/shogun/lib/observers/ParameterObserverLogger.cpp @@ -33,20 +33,26 @@ void CParameterObserverLogger::on_next_impl(const TimedObservedValue &value) { auto any_val = value.first->get_any(); auto pf_n = [&](auto v){ - SG_PRINT("[%l] Received a value called %s which contains: %s", convert_to_millis(value.second), + SG_PRINT("[%lu] Received a value called \"%s\" which contains: %s\n", convert_to_millis(value.second), name.c_str(), std::to_string(v).c_str()); }; auto pf_sgvector = [&](auto v){ - //SG_PRINT("[%l] Received a value called %s which contains: %s", convert_to_millis(value.second), - // name.c_str(), v.to_string().c_str()); + SG_PRINT("[%lu] Received a vector called \"%s\" which contains: %s\n", convert_to_millis(value.second), + name.c_str(), v.to_string().c_str()); }; auto pf_sgmatrix = [&](auto v){ - //SG_PRINT("[%l] Received a value called %s which contains: %s", convert_to_millis(value.second), - // name.c_str(), v.to_string().c_str()); + SG_PRINT("[%lu] Received a matrix called \"%s\" which contains: %s\n", convert_to_millis(value.second), + name.c_str(), v.to_string().c_str()); }; - sg_any_dispatch(any_val, sg_all_typemap, pf_n, pf_sgvector, pf_sgmatrix); - + try { + sg_any_dispatch(any_val, sg_all_typemap, pf_n, pf_sgvector, pf_sgmatrix); + } catch (ShogunException e) + { + SG_PRINT("[%lu] Received a value called \"%s\" which contains: %s\n", + convert_to_millis(value.second), name.c_str(), value.first->to_string().c_str() + ) + } } From dde303e1d789454c4eff977fcf949d24b607bb02 Mon Sep 17 00:00:00 2001 From: Giovanni De Toni Date: Tue, 23 Apr 2019 19:35:35 +0200 Subject: [PATCH 3/4] Move get_num_observations() to the put/get framework. --- src/shogun/lib/observers/ParameterObserver.cpp | 3 ++- src/shogun/lib/observers/ParameterObserver.h | 12 ++++++------ .../lib/observers/ParameterObserverCV_unittest.cc | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/shogun/lib/observers/ParameterObserver.cpp b/src/shogun/lib/observers/ParameterObserver.cpp index 6eba6c6a984..41f34c961be 100644 --- a/src/shogun/lib/observers/ParameterObserver.cpp +++ b/src/shogun/lib/observers/ParameterObserver.cpp @@ -42,6 +42,7 @@ using namespace shogun; ParameterObserver::ParameterObserver() : m_observed_parameters(), m_subscription_id(-1) { SG_ADD(&m_subscription_id, "subscription_id", "Id of the subscription to an object."); + this->watch_method("num_observations", &ParameterObserver::get_num_observations); } ParameterObserver::ParameterObserver(std::vector& parameters) @@ -72,7 +73,7 @@ bool ParameterObserver::filter(const std::string& param) return false; } -const index_t ParameterObserver::get_num_observations() const +index_t ParameterObserver::get_num_observations() const { return utils::safe_convert(m_observations.size()); }; \ No newline at end of file diff --git a/src/shogun/lib/observers/ParameterObserver.h b/src/shogun/lib/observers/ParameterObserver.h index bcc88dfd50c..4167567ce22 100644 --- a/src/shogun/lib/observers/ParameterObserver.h +++ b/src/shogun/lib/observers/ParameterObserver.h @@ -95,12 +95,6 @@ namespace shogun return this->m_observations[i].get(); }; - /** - * Get the total number of observation received. - * @return number of obsevation received. - */ - const index_t get_num_observations() const; - /** * Erase all observations registered so far by the observer. */ @@ -140,6 +134,12 @@ namespace shogun protected: + /** + * Get the total number of observation received. + * @return number of obsevation received. + */ + index_t get_num_observations() const; + /** * Implementation of the on_next method which will be needed * in order to process the observed value diff --git a/tests/unit/lib/observers/ParameterObserverCV_unittest.cc b/tests/unit/lib/observers/ParameterObserverCV_unittest.cc index 5c061da3534..a27fb7cfbd3 100644 --- a/tests/unit/lib/observers/ParameterObserverCV_unittest.cc +++ b/tests/unit/lib/observers/ParameterObserverCV_unittest.cc @@ -117,7 +117,7 @@ TEST(ParameterObserverCV, get_observations_locked) { std::shared_ptr par{generate(true)}; - for (size_t i = 0; i < par->get_num_observations(); i++) + for (index_t i = 0; i < par->get("num_observations"); i++) { auto name = par->get_observation(i)->get("name"); auto run = par->get_observation(i)->get(name); @@ -148,7 +148,7 @@ TEST(ParameterObserverCV, get_observations_unlocked) { std::shared_ptr par{generate(false)}; - for (size_t i = 0; i < par->get_num_observations(); i++) + for (index_t i = 0; i < par->get("num_observations"); i++) { auto name = par->get_observation(i)->get("name"); auto run = par->get_observation(i)->get(name); From 83fbb62d88c61d4f20c2111b51f92d79abd52429 Mon Sep 17 00:00:00 2001 From: Giovanni De Toni Date: Tue, 23 Apr 2019 22:59:17 +0200 Subject: [PATCH 4/4] Fix code style around Shogun, --- ...on_cross_validation_mkl_weight_storage.cpp | 2 +- src/shogun/base/SGObject.cpp | 29 +++-- src/shogun/base/SGObject.h | 111 +++++++++--------- src/shogun/base/base_types.h | 2 +- src/shogun/classifier/Perceptron.cpp | 2 +- src/shogun/evaluation/CrossValidation.cpp | 9 +- .../evaluation/CrossValidationStorage.cpp | 44 +++---- .../lib/observers/ParameterObserver.cpp | 12 +- src/shogun/lib/observers/ParameterObserver.h | 10 +- .../lib/observers/ParameterObserverCV.cpp | 30 +++-- .../lib/observers/ParameterObserverCV.h | 2 +- .../lib/observers/ParameterObserverLogger.cpp | 68 +++++++---- .../lib/observers/ParameterObserverLogger.h | 17 +-- .../modelselection/GradientModelSelection.cpp | 2 +- .../GridSearchModelSelection.cpp | 3 +- .../RandomSearchModelSelection.cpp | 3 +- tests/unit/base/SGObject_unittest.cc | 8 +- .../observers/ParameterObserverCV_unittest.cc | 26 ++-- 18 files changed, 219 insertions(+), 161 deletions(-) diff --git a/examples/undocumented/libshogun/evaluation_cross_validation_mkl_weight_storage.cpp b/examples/undocumented/libshogun/evaluation_cross_validation_mkl_weight_storage.cpp index b5ad27da846..6a7f9125656 100644 --- a/examples/undocumented/libshogun/evaluation_cross_validation_mkl_weight_storage.cpp +++ b/examples/undocumented/libshogun/evaluation_cross_validation_mkl_weight_storage.cpp @@ -55,7 +55,7 @@ SGMatrix calculate_weights( for (auto o : range(obs.get_num_observations())) { auto obs_storage = obs.get_observation(o); - for (auto i : range(obs_storage->getget("num_folds"))) { auto fold = obs_storage->get("folds", i); CMKLClassification* machine = diff --git a/src/shogun/base/SGObject.cpp b/src/shogun/base/SGObject.cpp index 5b1b34c144c..bed32571964 100644 --- a/src/shogun/base/SGObject.cpp +++ b/src/shogun/base/SGObject.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -22,7 +23,6 @@ #include #include #include -#include #include #include @@ -776,33 +776,36 @@ void CSGObject::subscribe(ParameterObserver* obs) // Create an observable which emits values only if they are about // parameters selected by the observable. - rxcpp::subscription subscription = m_observable_params - ->filter([obs](Some v) { - return obs->filter(v->get("name")); - }) - .timestamp() - .subscribe(sub); + rxcpp::subscription subscription = + m_observable_params + ->filter([obs](Some v) { + return obs->filter(v->get("name")); + }) + .timestamp() + .subscribe(sub); // Insert the subscription in the list m_subscriptions.insert( - std::make_pair( - std::move(m_next_subscription_index), - std::move(subscription))); + std::make_pair( + std::move(m_next_subscription_index), std::move(subscription))); obs->put("subscription_id", m_next_subscription_index); m_next_subscription_index++; } -void CSGObject::unsubscribe(ParameterObserver *obs) { +void CSGObject::unsubscribe(ParameterObserver* obs) +{ int64_t index = obs->get("subscription_id"); // Check if we have such subscription auto it = m_subscriptions.find(index); if (it == m_subscriptions.end()) - SG_ERROR("The object %s does not have any registered parameter observer with index %i", - this->get_name(), index); + SG_ERROR( + "The object %s does not have any registered parameter observer " + "with index %i", + this->get_name(), index); it->second.unsubscribe(); m_subscriptions.erase(index); diff --git a/src/shogun/base/SGObject.h b/src/shogun/base/SGObject.h index c9fed87ca1a..15bc22cc40e 100644 --- a/src/shogun/base/SGObject.h +++ b/src/shogun/base/SGObject.h @@ -438,7 +438,8 @@ class CSGObject } #ifndef SWIG - /** Typed array getter for an object array class parameter of a Shogun base class + /** Typed array getter for an object array class parameter of a Shogun base + * class * type, identified by a name and an index. * * Returns nullptr if parameter of desired type does not exist. @@ -448,7 +449,7 @@ class CSGObject * @return desired element */ template ::value>::type> + class X = typename std::enable_if::value>::type> T* get(const std::string& name, index_t index, std::nothrow_t) const { CSGObject* result = nullptr; @@ -467,14 +468,15 @@ class CSGObject } template ::value>::type> + class X = typename std::enable_if::value>::type> T* get(const std::string& name, index_t index) const { auto result = this->get(name, index, std::nothrow); - if (!result) { - SG_ERROR("Could not get array parameter %s::%s[%d] of type %s\n", - get_name(), name.c_str(), index, demangled_type().c_str()); - + if (!result) + { + SG_ERROR( + "Could not get array parameter %s::%s[%d] of type %s\n", + get_name(), name.c_str(), index, demangled_type().c_str()); } return result; }; @@ -673,7 +675,8 @@ class CSGObject /** * Detach an observer from the current SGObject. - * @param subscription_index the index obtained by calling the subscribe procedure + * @param subscription_index the index obtained by calling the subscribe + * procedure */ void unsubscribe(ParameterObserver* obs); @@ -967,7 +970,6 @@ class CSGObject Unique param_obs_list; protected: - /** * Return total subscriptions * @return total number of subscriptions @@ -1014,70 +1016,69 @@ class CSGObject void register_observable( const std::string& name, const std::string& description); - /** - * Get the current step for the observed values. - */ +/** + * Get the current step for the observed values. + */ #ifndef SWIG - SG_FORCED_INLINE int64_t get_step() const - { - int64_t step = -1; - Tag tag("current_iteration"); - if (has(tag)) + SG_FORCED_INLINE int64_t get_step() const { - step = get(tag); + int64_t step = -1; + Tag tag("current_iteration"); + if (has(tag)) + { + step = get(tag); + } + return step; } - return step; - } #endif - /** mapping from strings to enum for SWIG interface */ - stringToEnumMapType m_string_to_enum_map; + /** mapping from strings to enum for SWIG interface */ + stringToEnumMapType m_string_to_enum_map; -public: - /** io */ - SGIO* io; + public: + /** io */ + SGIO* io; - /** parallel */ - Parallel* parallel; + /** parallel */ + Parallel* parallel; - /** version */ - Version* version; + /** version */ + Version* version; - /** parameters */ - Parameter* m_parameters; + /** parameters */ + Parameter* m_parameters; - /** model selection parameters */ - Parameter* m_model_selection_parameters; + /** model selection parameters */ + Parameter* m_model_selection_parameters; - /** parameters wrt which we can compute gradients */ - Parameter* m_gradient_parameters; + /** parameters wrt which we can compute gradients */ + Parameter* m_gradient_parameters; - /** Hash of parameter values*/ - uint32_t m_hash; + /** Hash of parameter values*/ + uint32_t m_hash; -private: - - EPrimitiveType m_generic; - bool m_load_pre_called; - bool m_load_post_called; - bool m_save_pre_called; - bool m_save_post_called; + private: + EPrimitiveType m_generic; + bool m_load_pre_called; + bool m_load_post_called; + bool m_save_pre_called; + bool m_save_post_called; - RefCount* m_refcount; + RefCount* m_refcount; - /** Subject used to create the params observer */ - SGSubject* m_subject_params; + /** Subject used to create the params observer */ + SGSubject* m_subject_params; - /** Parameter Observable */ - SGObservable* m_observable_params; + /** Parameter Observable */ + SGObservable* m_observable_params; - /** Subscriber used to call onNext, onComplete etc.*/ - SGSubscriber* m_subscriber_params; + /** Subscriber used to call onNext, onComplete etc.*/ + SGSubscriber* m_subscriber_params; - /** List of subscription for this SGObject */ - std::map m_subscriptions; - int64_t m_next_subscription_index; -}; + /** List of subscription for this SGObject */ + std::map m_subscriptions; + int64_t m_next_subscription_index; + }; #ifndef SWIG #ifndef DOXYGEN_SHOULD_SKIP_THIS diff --git a/src/shogun/base/base_types.h b/src/shogun/base/base_types.h index a70ac6ba2e5..12abce4450f 100644 --- a/src/shogun/base/base_types.h +++ b/src/shogun/base/base_types.h @@ -54,7 +54,7 @@ namespace shogun std::is_same::value || std::is_same::value || std::is_same::value || - std::is_same::value> + std::is_same::value> { }; diff --git a/src/shogun/classifier/Perceptron.cpp b/src/shogun/classifier/Perceptron.cpp index ffc1af62d18..6ec054cbd3c 100644 --- a/src/shogun/classifier/Perceptron.cpp +++ b/src/shogun/classifier/Perceptron.cpp @@ -73,7 +73,7 @@ void CPerceptron::iteration() { converged = false; const auto gradient = learn_rate * true_label; - put("bias", bias+gradient); + put("bias", bias + gradient); v.add(gradient, w); put("w", w); } diff --git a/src/shogun/evaluation/CrossValidation.cpp b/src/shogun/evaluation/CrossValidation.cpp index c67f98c87ec..14fd8982d67 100644 --- a/src/shogun/evaluation/CrossValidation.cpp +++ b/src/shogun/evaluation/CrossValidation.cpp @@ -99,7 +99,9 @@ CEvaluationResult* CCrossValidation::evaluate_impl() CrossValidationStorage* storage = new CrossValidationStorage(); SG_REF(storage) storage->put("num_runs", utils::safe_convert(m_num_runs)); - storage->put("num_folds", utils::safe_convert(m_splitting_strategy->get_num_subsets())); + storage->put( + "num_folds", utils::safe_convert( + m_splitting_strategy->get_num_subsets())); storage->put("labels", m_labels); storage->post_init(); SG_DEBUG("Ending CrossValidationStorage initilization.\n") @@ -109,8 +111,9 @@ CEvaluationResult* CCrossValidation::evaluate_impl() SG_DEBUG("result of cross-validation run %d is %f\n", i, results[i]) /* Emit the value */ - observe(i, "cross_validation_run", "One run of CrossValidation", - storage->as()); + observe( + i, "cross_validation_run", "One run of CrossValidation", + storage->as()); SG_UNREF(storage) } diff --git a/src/shogun/evaluation/CrossValidationStorage.cpp b/src/shogun/evaluation/CrossValidationStorage.cpp index a1b111bf2d4..5483d6c92f8 100644 --- a/src/shogun/evaluation/CrossValidationStorage.cpp +++ b/src/shogun/evaluation/CrossValidationStorage.cpp @@ -50,8 +50,8 @@ CrossValidationFoldStorage::CrossValidationFoldStorage() : CEvaluationResult() m_test_true_result = NULL; SG_ADD( - &m_current_run_index, "run_index", - "The current run index of this fold", ParameterProperties::HYPER); + &m_current_run_index, "run_index", "The current run index of this fold", + ParameterProperties::HYPER); SG_ADD( &m_current_fold_index, "fold_index", "The current fold index", ParameterProperties::HYPER); @@ -59,18 +59,20 @@ CrossValidationFoldStorage::CrossValidationFoldStorage() : CEvaluationResult() &m_trained_machine, "trained_machine", "The machine trained by this fold", ParameterProperties::HYPER); SG_ADD( - &m_test_result, "predicted_labels", - "The test result of this fold", ParameterProperties::HYPER); + &m_test_result, "predicted_labels", "The test result of this fold", + ParameterProperties::HYPER); SG_ADD( &m_test_true_result, "ground_truth_labels", "The true test result for this fold", ParameterProperties::HYPER); - SG_ADD(&m_train_indices, "train_indices", - "Indices used for training", ParameterProperties::HYPER); - SG_ADD(&m_test_indices, "test_indices", - "Indices used for testing", ParameterProperties::HYPER); - SG_ADD(&m_evaluation_result, "evaluation_result", - "Result of the evaluation", ParameterProperties::HYPER); - + SG_ADD( + &m_train_indices, "train_indices", "Indices used for training", + ParameterProperties::HYPER); + SG_ADD( + &m_test_indices, "test_indices", "Indices used for testing", + ParameterProperties::HYPER); + SG_ADD( + &m_evaluation_result, "evaluation_result", "Result of the evaluation", + ParameterProperties::HYPER); } CrossValidationFoldStorage::~CrossValidationFoldStorage() @@ -84,8 +86,8 @@ void CrossValidationFoldStorage::post_update_results() { } -void CrossValidationFoldStorage::print_result() { - +void CrossValidationFoldStorage::print_result() +{ } /** CrossValidationStorage **/ @@ -100,12 +102,14 @@ CrossValidationStorage::CrossValidationStorage() : CEvaluationResult() &m_num_runs, "num_runs", "The total number of cross-validation runs", ParameterProperties::HYPER); SG_ADD( - &m_num_folds, "num_folds", - "The total number of cross-validation folds", ParameterProperties::HYPER); + &m_num_folds, "num_folds", "The total number of cross-validation folds", + ParameterProperties::HYPER); SG_ADD( - &m_original_labels, "labels", - "The labels used for this cross-validation", ParameterProperties::HYPER); - this->watch_param("folds", &m_folds_results, AnyParameterProperties("Fold results")); + &m_original_labels, "labels", + "The labels used for this cross-validation", + ParameterProperties::HYPER); + this->watch_param( + "folds", &m_folds_results, AnyParameterProperties("Fold results")); } CrossValidationStorage::~CrossValidationStorage() @@ -126,6 +130,6 @@ void CrossValidationStorage::append_fold_result( m_folds_results.push_back(result); } -void CrossValidationStorage::print_result() { - +void CrossValidationStorage::print_result() +{ } diff --git a/src/shogun/lib/observers/ParameterObserver.cpp b/src/shogun/lib/observers/ParameterObserver.cpp index 41f34c961be..fe8ffea3de4 100644 --- a/src/shogun/lib/observers/ParameterObserver.cpp +++ b/src/shogun/lib/observers/ParameterObserver.cpp @@ -32,17 +32,21 @@ * Written (W) 2017 Giovanni De Toni * */ +#include #include #include -#include #include using namespace shogun; -ParameterObserver::ParameterObserver() : m_observed_parameters(), m_subscription_id(-1) +ParameterObserver::ParameterObserver() + : m_observed_parameters(), m_subscription_id(-1) { - SG_ADD(&m_subscription_id, "subscription_id", "Id of the subscription to an object."); - this->watch_method("num_observations", &ParameterObserver::get_num_observations); + SG_ADD( + &m_subscription_id, "subscription_id", + "Id of the subscription to an object."); + this->watch_method( + "num_observations", &ParameterObserver::get_num_observations); } ParameterObserver::ParameterObserver(std::vector& parameters) diff --git a/src/shogun/lib/observers/ParameterObserver.h b/src/shogun/lib/observers/ParameterObserver.h index 4167567ce22..8dbe16acb5d 100644 --- a/src/shogun/lib/observers/ParameterObserver.h +++ b/src/shogun/lib/observers/ParameterObserver.h @@ -89,9 +89,13 @@ namespace shogun * @param i the index * @return the observation casted to the requested type */ - ObservedValue * get_observation(index_t i) + ObservedValue* get_observation(index_t i) { - REQUIRE(i>=0 && iget_num_observations(), "Observation index (%i) is out of bound (total observations %i)", i, this->get_num_observations()); + REQUIRE( + i >= 0 && i < this->get_num_observations(), + "Observation index (%i) is out of bound (total observations " + "%i)", + i, this->get_num_observations()); return this->m_observations[i].get(); }; @@ -133,7 +137,6 @@ namespace shogun } protected: - /** * Get the total number of observation received. * @return number of obsevation received. @@ -161,7 +164,6 @@ namespace shogun * Subscription id set when I subscribe to a machine */ int64_t m_subscription_id; - }; } diff --git a/src/shogun/lib/observers/ParameterObserverCV.cpp b/src/shogun/lib/observers/ParameterObserverCV.cpp index 22c0f3a5900..e1d3aa94794 100644 --- a/src/shogun/lib/observers/ParameterObserverCV.cpp +++ b/src/shogun/lib/observers/ParameterObserverCV.cpp @@ -56,8 +56,8 @@ CParameterObserverCV::~CParameterObserverCV() void CParameterObserverCV::on_next_impl(const shogun::TimedObservedValue& value) { CrossValidationStorage* recalled_value = - dynamic_cast(value.first->get( - value.first->get("name"))); + dynamic_cast( + value.first->get(value.first->get("name"))); SG_REF(recalled_value); /* Print information on screen if enabled*/ @@ -78,17 +78,25 @@ void CParameterObserverCV::print_observed_value( { for (index_t i = 0; i < value->get("num_folds"); i++) { - auto f = value->get("folds",i); + auto f = value->get("folds", i); SG_PRINT("\n") - SG_PRINT("Current run index: %i\n", f->get("current_run_index")) - SG_PRINT("Current fold index: %i\n", f->get("current_fold_index")) - f->get>("train_indices").display_vector("Train Indices "); - f->get>("test_indices").display_vector("Test Indices "); + SG_PRINT( + "Current run index: %i\n", f->get("current_run_index")) + SG_PRINT( + "Current fold index: %i\n", f->get("current_fold_index")) + f->get>("train_indices") + .display_vector("Train Indices "); + f->get>("test_indices") + .display_vector("Test Indices "); print_machine_information(f->get("trained_machine")); - f->get("test_result")->get_values().display_vector("Test Labels "); - f->get("test_true_result")->get_values().display_vector( - "Test True Label "); - SG_PRINT("Evaluation result: %f\n", f->get("evaluation_result")); + f->get("test_result") + ->get_values() + .display_vector("Test Labels "); + f->get("test_true_result") + ->get_values() + .display_vector("Test True Label "); + SG_PRINT( + "Evaluation result: %f\n", f->get("evaluation_result")); SG_UNREF(f) } } diff --git a/src/shogun/lib/observers/ParameterObserverCV.h b/src/shogun/lib/observers/ParameterObserverCV.h index 8b35f41a930..5dfaa0a8dc9 100644 --- a/src/shogun/lib/observers/ParameterObserverCV.h +++ b/src/shogun/lib/observers/ParameterObserverCV.h @@ -38,8 +38,8 @@ #include #include -#include #include +#include namespace shogun { diff --git a/src/shogun/lib/observers/ParameterObserverLogger.cpp b/src/shogun/lib/observers/ParameterObserverLogger.cpp index 73e3685d8b8..7ed89de4057 100644 --- a/src/shogun/lib/observers/ParameterObserverLogger.cpp +++ b/src/shogun/lib/observers/ParameterObserverLogger.cpp @@ -5,54 +5,70 @@ * */ -#include #include +#include using namespace shogun; -CParameterObserverLogger::CParameterObserverLogger() {} - -CParameterObserverLogger::CParameterObserverLogger(std::vector ¶meters) : ParameterObserver( - parameters) {} - -CParameterObserverLogger::~CParameterObserverLogger() { - +CParameterObserverLogger::CParameterObserverLogger() +{ } -void CParameterObserverLogger::on_error(std::exception_ptr ptr) { +CParameterObserverLogger::CParameterObserverLogger( + std::vector& parameters) + : ParameterObserver(parameters) +{ +} +CParameterObserverLogger::~CParameterObserverLogger() +{ } -void CParameterObserverLogger::on_complete() { +void CParameterObserverLogger::on_error(std::exception_ptr ptr) +{ +} +void CParameterObserverLogger::on_complete() +{ } -void CParameterObserverLogger::on_next_impl(const TimedObservedValue &value) { +void CParameterObserverLogger::on_next_impl(const TimedObservedValue& value) +{ auto name = value.first->get("name"); auto any_val = value.first->get_any(); - auto pf_n = [&](auto v){ - SG_PRINT("[%lu] Received a value called \"%s\" which contains: %s\n", convert_to_millis(value.second), - name.c_str(), std::to_string(v).c_str()); + auto pf_n = [&](auto v) { + SG_PRINT( + "[%lu] Received a value called \"%s\" which contains: %s\n", + convert_to_millis(value.second), name.c_str(), + std::to_string(v).c_str()); }; - auto pf_sgvector = [&](auto v){ - SG_PRINT("[%lu] Received a vector called \"%s\" which contains: %s\n", convert_to_millis(value.second), - name.c_str(), v.to_string().c_str()); + auto pf_sgvector = [&](auto v) { + SG_PRINT( + "[%lu] Received a vector called \"%s\" which contains: %s\n", + convert_to_millis(value.second), name.c_str(), + v.to_string().c_str()); }; - auto pf_sgmatrix = [&](auto v){ - SG_PRINT("[%lu] Received a matrix called \"%s\" which contains: %s\n", convert_to_millis(value.second), - name.c_str(), v.to_string().c_str()); + auto pf_sgmatrix = [&](auto v) { + SG_PRINT( + "[%lu] Received a matrix called \"%s\" which contains: %s\n", + convert_to_millis(value.second), name.c_str(), + v.to_string().c_str()); }; - try { - sg_any_dispatch(any_val, sg_all_typemap, pf_n, pf_sgvector, pf_sgmatrix); - } catch (ShogunException e) + try + { + sg_any_dispatch( + any_val, sg_all_typemap, pf_n, pf_sgvector, pf_sgmatrix); + } + catch (ShogunException e) { - SG_PRINT("[%lu] Received a value called \"%s\" which contains: %s\n", - convert_to_millis(value.second), name.c_str(), value.first->to_string().c_str() - ) + SG_PRINT( + "[%lu] Received a value called \"%s\" which contains: %s\n", + convert_to_millis(value.second), name.c_str(), + value.first->to_string().c_str()) } } diff --git a/src/shogun/lib/observers/ParameterObserverLogger.h b/src/shogun/lib/observers/ParameterObserverLogger.h index b70740c763b..10a9173b51e 100644 --- a/src/shogun/lib/observers/ParameterObserverLogger.h +++ b/src/shogun/lib/observers/ParameterObserverLogger.h @@ -9,17 +9,19 @@ #include -namespace shogun { +namespace shogun +{ /** * This class implements a logger which prints all observed updates. */ - class CParameterObserverLogger : public ParameterObserver { + class CParameterObserverLogger : public ParameterObserver + { public: CParameterObserverLogger(); - CParameterObserverLogger(std::vector ¶meters); + CParameterObserverLogger(std::vector& parameters); virtual ~CParameterObserverLogger(); @@ -27,15 +29,14 @@ namespace shogun { virtual void on_complete(); - virtual const char *get_name() const { + virtual const char* get_name() const + { return "ParameterObserverLogger"; }; protected: - virtual void on_next_impl(const TimedObservedValue &value); - + virtual void on_next_impl(const TimedObservedValue& value); }; } - -#endif //SHOGUN_PARAMETEROBSERVERLOGGER_H +#endif // SHOGUN_PARAMETEROBSERVERLOGGER_H diff --git a/src/shogun/modelselection/GradientModelSelection.cpp b/src/shogun/modelselection/GradientModelSelection.cpp index 69523670ff4..5d25ea7561c 100644 --- a/src/shogun/modelselection/GradientModelSelection.cpp +++ b/src/shogun/modelselection/GradientModelSelection.cpp @@ -173,7 +173,7 @@ float64_t CGradientModelSelection::get_cost(SGVector model_vars, SGVe // evaluate the machine CEvaluationResult* evaluation_result=m_machine_eval->evaluate(); - CGradientResult* gradient_result=evaluation_result->as(); + CGradientResult* gradient_result = evaluation_result->as(); SG_REF(gradient_result); SG_UNREF(evaluation_result); diff --git a/src/shogun/modelselection/GridSearchModelSelection.cpp b/src/shogun/modelselection/GridSearchModelSelection.cpp index d369cbf4d15..c3177338249 100644 --- a/src/shogun/modelselection/GridSearchModelSelection.cpp +++ b/src/shogun/modelselection/GridSearchModelSelection.cpp @@ -72,7 +72,8 @@ CParameterCombination* CGridSearchModelSelection::select_model(bool print_state) machine->m_model_selection_parameters); /* note that this may implicitly lock and unlockthe machine */ - CCrossValidationResult* result= m_machine_eval->evaluate()->as(); + CCrossValidationResult* result = + m_machine_eval->evaluate()->as(); if (print_state) result->print_result(); diff --git a/src/shogun/modelselection/RandomSearchModelSelection.cpp b/src/shogun/modelselection/RandomSearchModelSelection.cpp index f23cfe19622..7234ecd2f76 100644 --- a/src/shogun/modelselection/RandomSearchModelSelection.cpp +++ b/src/shogun/modelselection/RandomSearchModelSelection.cpp @@ -83,7 +83,8 @@ CParameterCombination* CRandomSearchModelSelection::select_model(bool print_stat machine->m_model_selection_parameters); /* note that this may implicitly lock and unlockthe machine */ - CCrossValidationResult* result = m_machine_eval->evaluate()->as(); + CCrossValidationResult* result = + m_machine_eval->evaluate()->as(); if (print_state) result->print_result(); diff --git a/tests/unit/base/SGObject_unittest.cc b/tests/unit/base/SGObject_unittest.cc index ac80bf87ea4..50d3110ad20 100644 --- a/tests/unit/base/SGObject_unittest.cc +++ b/tests/unit/base/SGObject_unittest.cc @@ -577,7 +577,9 @@ TEST(SGObject, subscribe_observer) obj->subscribe(param_obs); EXPECT_EQ(param_obs->get("subscription_id"), 0); - EXPECT_EQ(obj->get("num_subscriptions"), utils::safe_convert(1)); + EXPECT_EQ( + obj->get("num_subscriptions"), + utils::safe_convert(1)); } TEST(SGObject, unsubscribe_observer) @@ -588,7 +590,9 @@ TEST(SGObject, unsubscribe_observer) obj->unsubscribe(param_obs); EXPECT_EQ(param_obs->get("subscription_id"), -1); - EXPECT_EQ(obj->get("num_subscriptions"), utils::safe_convert(0)); + EXPECT_EQ( + obj->get("num_subscriptions"), + utils::safe_convert(0)); } TEST(SGObject, unsubscribe_observer_failure) diff --git a/tests/unit/lib/observers/ParameterObserverCV_unittest.cc b/tests/unit/lib/observers/ParameterObserverCV_unittest.cc index a27fb7cfbd3..070f553597a 100644 --- a/tests/unit/lib/observers/ParameterObserverCV_unittest.cc +++ b/tests/unit/lib/observers/ParameterObserverCV_unittest.cc @@ -132,11 +132,16 @@ TEST(ParameterObserverCV, get_observations_locked) SG_REF(fold) EXPECT_EQ(fold->get("run_index"), i); EXPECT_EQ(fold->get("fold_index"), j); - EXPECT_TRUE(fold->get>("train_indices").size() != 0); - EXPECT_TRUE(fold->get>("test_indices").size() != 0); + EXPECT_TRUE( + fold->get>("train_indices").size() != 0); + EXPECT_TRUE( + fold->get>("test_indices").size() != 0); EXPECT_TRUE(fold->get("trained_machine") != NULL); - EXPECT_TRUE(fold->get("predicted_labels")->get_num_labels() != 0); - EXPECT_TRUE(fold->get("ground_truth_labels")->get_num_labels() != 0); + EXPECT_TRUE( + fold->get("predicted_labels")->get_num_labels() != 0); + EXPECT_TRUE( + fold->get("ground_truth_labels")->get_num_labels() != + 0); EXPECT_TRUE(fold->get("evaluation_result") != 0); SG_UNREF(fold) } @@ -163,11 +168,16 @@ TEST(ParameterObserverCV, get_observations_unlocked) SG_REF(fold) EXPECT_EQ(fold->get("run_index"), i); EXPECT_EQ(fold->get("fold_index"), j); - EXPECT_TRUE(fold->get>("train_indices").size() != 0); - EXPECT_TRUE(fold->get>("test_indices").size() != 0); + EXPECT_TRUE( + fold->get>("train_indices").size() != 0); + EXPECT_TRUE( + fold->get>("test_indices").size() != 0); EXPECT_TRUE(fold->get("trained_machine") != NULL); - EXPECT_TRUE(fold->get("predicted_labels")->get_num_labels() != 0); - EXPECT_TRUE(fold->get("ground_truth_labels")->get_num_labels() != 0); + EXPECT_TRUE( + fold->get("predicted_labels")->get_num_labels() != 0); + EXPECT_TRUE( + fold->get("ground_truth_labels")->get_num_labels() != + 0); EXPECT_TRUE(fold->get("evaluation_result") != 0); SG_UNREF(fold) }