Use visitors in put and get (#5037)

* put get visitor * replace std::bind with lambda * clean up parameter properties * clean up repetitive checks for parameter existence
shogun-toolbox · May 22, 2020 · 93a649b · 93a649b
1 parent e29377d
commit 93a649b
Show file tree

Hide file tree

Showing 7 changed files with 242 additions and 165 deletions.
diff --git a/.ci/ci.yml b/.ci/ci.yml
@@ -62,6 +62,7 @@ jobs:
 - job: swig
   displayName: Linux SWIG Interface
   dependsOn: libshogun
+  timeoutInMinutes: 90
 
   pool:
     vmImage: ubuntu-16.04

diff --git a/src/shogun/base/AnyParameter.h b/src/shogun/base/AnyParameter.h
@@ -39,12 +39,17 @@ namespace shogun
 		// has automatically computed value
 		AUTO = 1u << 10,
 
+		// is const
 		READONLY = 1u << 11,
 
-		// an executable function
-		RUNFUNCTION = 1u << 12,
+		// a class member function with side effects
+		FUNCTION = 1u << 12,
 
-		CONSTRAIN = 1u << 13,
+		// a const class member function
+		CONSTFUNCTION = 1u << 13,
+
+		// parameter is constrained, e.g. has to be positive
+		CONSTRAIN = 1u << 14,
 
 		ALL = std::numeric_limits<uint32_t>::max(),
 	};
@@ -58,7 +63,8 @@ namespace shogun
 	        {ParameterProperties::SETTING, "SETTING"},
 	        {ParameterProperties::AUTO, "AUTO"},
 	        {ParameterProperties::READONLY, "READONLY"},
-	        {ParameterProperties::RUNFUNCTION, "RUNFUNCTION"},
+	        {ParameterProperties::FUNCTION, "FUNCTION"},
+	        {ParameterProperties::CONSTFUNCTION, "CONSTFUNCTION"},
 	        {ParameterProperties::CONSTRAIN, "CONSTRAIN"}};
 
 	enableEnumClassBitmask(ParameterProperties);
@@ -182,7 +188,12 @@ namespace shogun
 		{
 		}
 
-		Any get_value() const
+		const Any& get_value() const
+		{
+			return m_value;
+		}
+
+		Any& get_value()
 		{
 			return m_value;
 		}

diff --git a/src/shogun/base/SGObject.cpp b/src/shogun/base/SGObject.cpp
@@ -359,69 +359,69 @@ void SGObject::create_parameter(
 	self->create(std::forward<BaseTag>(_tag), std::forward<AnyParameter>(parameter));
 }
 
-void SGObject::update_parameter(const BaseTag& _tag, const Any& value, bool do_checks)
+const AnyParameter& SGObject::get_parameter(const BaseTag& _tag) const
 {
-	auto& param = self->at(_tag);
-	auto& pprop = param.get_properties();
-	if (pprop.has_property(ParameterProperties::READONLY))
-	        require(!do_checks,
-			"{}::{} is marked as read-only and cannot be modified!",
-	          	get_name(), _tag.name().c_str());
-
-	if (pprop.has_property(ParameterProperties::CONSTRAIN))
+	if (!has_parameter(_tag))
 	{
-		auto msg = self->find(_tag)->second.get_constrain_function()(value);
-		if (!msg.empty())
-		{
-			require(!do_checks,
-				"{}::{} cannot be updated because it must be: {}!",
-				get_name(), _tag.name().c_str(), msg.c_str());
-		}
+		error(
+			"Parameter {}::{} does not exist.", get_name(),
+			_tag.name().c_str());
 	}
-	param.set_value(value);
-	for (auto& method : param.get_callbacks())
-		method();
-
-	pprop.remove_property(ParameterProperties::AUTO);
-}
-
-AnyParameter SGObject::get_parameter(const BaseTag& _tag) const
-{
-	const auto& parameter = self->get(_tag);
+
+	const auto& parameter = self->at(_tag);
+
 	if (parameter.get_properties().has_property(
-	        ParameterProperties::RUNFUNCTION))
+	        ParameterProperties::FUNCTION))
 	{
 		error(
 		    "The parameter {}::{} is registered as a function, "
 		    "use the .run() method instead!\n",
 		    get_name(), _tag.name().c_str());
 	}
-	if (parameter.get_value().empty())
+	return parameter;
+}
+
+AnyParameter& SGObject::get_parameter(const BaseTag& _tag)
+{
+	if (!has_parameter(_tag))
+	{
+		error(
+			"Parameter {}::{} does not exist.", get_name(),
+			_tag.name().c_str());
+	}
+
+	auto& parameter = self->at(_tag);
+
+	if (parameter.get_properties().has_property(
+	        ParameterProperties::FUNCTION))
 	{
 		error(
-		    "There is no parameter called \"{}\" in {}", _tag.name().c_str(),
-		    get_name());
+		    "The parameter {}::{} is registered as a function, "
+		    "use the .run() method instead!\n",
+		    get_name(), _tag.name().c_str());
 	}
 	return parameter;
 }
 
-AnyParameter SGObject::get_function(const BaseTag& _tag) const
+const AnyParameter& SGObject::get_function(const BaseTag& _tag) const
 {
-	const auto& parameter = self->get(_tag);
+	if (!has_parameter(_tag))
+	{
+		error(
+			"Parameter {}::{} does not exist.", get_name(),
+			_tag.name().c_str());
+	}
+
+	const auto& parameter = self->at(_tag);
+
 	if (!parameter.get_properties().has_property(
-	        ParameterProperties::RUNFUNCTION))
+	        ParameterProperties::FUNCTION))
 	{
 		error(
 		    "The parameter {}::{} is not registered as a function, "
 		    "use the .get() method instead",
 		    get_name(), _tag.name().c_str());
 	}
-	if (parameter.get_value().empty())
-	{
-		error(
-		    "There is no parameter called \"{}\" in {}", _tag.name().c_str(),
-		    get_name());
-	}
 	return parameter;
 }