Use `UpdateAllowUnknown' for non-model related parameter. (#4961)

* Use `UpdateAllowUnknown' for non-model related parameter.

Model parameter can not pack an additional boolean value due to binary IO
format.  This commit deals only with non-model related parameter configuration.

* Add tidy command line arg for use-dmlc-gtest.

tests/ci_build/tidy.py

@@ -26,17 +26,22 @@ def call(args):
 
 
 class ClangTidy(object):
-    '''
-    clang tidy wrapper.
+    ''' clang tidy wrapper.
     Args:
-      cpp_lint: Run linter on C++ source code.
-      cuda_lint: Run linter on CUDA source code.
+      args:  Command line arguments.
+          cpp_lint: Run linter on C++ source code.
+          cuda_lint: Run linter on CUDA source code.
+          use_dmlc_gtest: Whether to use gtest bundled in dmlc-core.
     '''
-    def __init__(self, cpp_lint, cuda_lint):
-        self.cpp_lint = cpp_lint
-        self.cuda_lint = cuda_lint
+    def __init__(self, args):
+        self.cpp_lint = args.cpp
+        self.cuda_lint = args.cuda
+        self.use_dmlc_gtest = args.use_dmlc_gtest
+
         print('Run linter on CUDA: ', self.cuda_lint)
         print('Run linter on C++:', self.cpp_lint)
+        print('Use dmlc gtest:', self.use_dmlc_gtest)
+
         if not self.cpp_lint and not self.cuda_lint:
             raise ValueError('Both --cpp and --cuda are set to 0.')
         self.root_path = os.path.abspath(os.path.curdir)
@@ -58,7 +63,12 @@ class ClangTidy(object):
         os.mkdir(self.cdb_path)
         os.chdir(self.cdb_path)
         cmake_args = ['cmake', '..', '-DCMAKE_EXPORT_COMPILE_COMMANDS=ON',
-                      '-DGOOGLE_TEST=ON', '-DUSE_DMLC_GTEST=ON']
+                      '-DGOOGLE_TEST=ON']
+        if self.use_dmlc_gtest:
+            cmake_args.append('-DUSE_DMLC_GTEST=ON')
+        else:
+            cmake_args.append('-DUSE_DMLC_GTEST=OFF')
+
         if self.cuda_lint:
             cmake_args.extend(['-DUSE_CUDA=ON', '-DUSE_NCCL=ON'])
         subprocess.run(cmake_args)
@@ -234,11 +244,13 @@ if __name__ == '__main__':
     parser = argparse.ArgumentParser(description='Run clang-tidy.')
     parser.add_argument('--cpp', type=int, default=1)
     parser.add_argument('--cuda', type=int, default=1)
+    parser.add_argument('--use-dmlc-gtest', type=int, default=1,
+                        help='Whether to use gtest bundled in dmlc-core.')
     args = parser.parse_args()
 
     test_tidy()
 
-    with ClangTidy(args.cpp, args.cuda) as linter:
+    with ClangTidy(args) as linter:
         passed = linter.run()
     if not passed:
         sys.exit(1)

tests/cpp/common/test_monitor.cc

@@ -16,6 +16,8 @@ TEST(Monitor, Logging) {
 
   Args args = {std::make_pair("verbosity", "3")};
   ConsoleLogger::Configure(args);
+  ASSERT_EQ(ConsoleLogger::GlobalVerbosity(), ConsoleLogger::LogVerbosity::kDebug);
+
   testing::internal::CaptureStderr();
   run_monitor();
   std::string output = testing::internal::GetCapturedStderr();
@@ -28,6 +30,8 @@ TEST(Monitor, Logging) {
   run_monitor();
   output = testing::internal::GetCapturedStderr();
   ASSERT_EQ(output.size(), 0);
+
+  ConsoleLogger::Configure(Args{{"verbosity", "1"}});
 }
 }  // namespace common
 }  // namespace xgboost

tests/cpp/common/test_parameter.cc

@@ -1,5 +1,9 @@
-#include <dmlc/parameter.h>
+/*!
+ * Copyright (c) by Contributors 2019
+ */
 #include <gtest/gtest.h>
+
+#include <xgboost/base.h>
 #include <xgboost/parameter.h>
 
 enum class Foo : int {
@@ -8,10 +12,10 @@ enum class Foo : int {
 
 DECLARE_FIELD_ENUM_CLASS(Foo);
 
-struct MyParam : dmlc::Parameter<MyParam> {
+struct MyEnumParam : xgboost::XGBoostParameter<MyEnumParam> {
   Foo foo;
   int bar;
-  DMLC_DECLARE_PARAMETER(MyParam) {
+  DMLC_DECLARE_PARAMETER(MyEnumParam) {
     DMLC_DECLARE_FIELD(foo)
       .set_default(Foo::kBar)
       .add_enum("bar", Foo::kBar)
@@ -23,10 +27,10 @@ struct MyParam : dmlc::Parameter<MyParam> {
   }
 };
 
-DMLC_REGISTER_PARAMETER(MyParam);
+DMLC_REGISTER_PARAMETER(MyEnumParam);
 
 TEST(EnumClassParam, Basic) {
-  MyParam param;
+  MyEnumParam param;
   std::map<std::string, std::string> kwargs{
     {"foo", "frog"}, {"bar", "10"}
   };
@@ -53,3 +57,44 @@ TEST(EnumClassParam, Basic) {
   kwargs["foo"] = "human";
   ASSERT_THROW(param.Init(kwargs), dmlc::ParamError);
 }
+
+struct UpdatableParam : xgboost::XGBoostParameter<UpdatableParam> {
+  float f { 0.0f };
+  double d { 0.0 };
+
+  DMLC_DECLARE_PARAMETER(UpdatableParam) {
+    DMLC_DECLARE_FIELD(f)
+        .set_default(11.0f);
+    DMLC_DECLARE_FIELD(d)
+        .set_default(2.71828f);
+  }
+};
+
+DMLC_REGISTER_PARAMETER(UpdatableParam);
+
+TEST(XGBoostParameter, Update) {
+  {
+    UpdatableParam p;
+    auto constexpr kRtEps = xgboost::kRtEps;
+
+    p.UpdateAllowUnknown(xgboost::Args{});
+    // When it's not initialized, perform set_default.
+    ASSERT_NEAR(p.f, 11.0f, kRtEps);
+    ASSERT_NEAR(p.d, 2.71828f, kRtEps);
+
+    p.d = 3.14149;
+
+    p.UpdateAllowUnknown(xgboost::Args{{"f", "2.71828"}});
+    ASSERT_NEAR(p.f, 2.71828f, kRtEps);
+
+    // p.d is un-effected by the update.
+    ASSERT_NEAR(p.d, 3.14149, kRtEps);
+  }
+  {
+    UpdatableParam p;
+    auto constexpr kRtEps = xgboost::kRtEps;
+    p.UpdateAllowUnknown(xgboost::Args{{"f", "2.71828"}});
+    ASSERT_NEAR(p.f, 2.71828f, kRtEps);
+    ASSERT_NEAR(p.d, 2.71828, kRtEps);  // default
+  }
+}

tests/cpp/gbm/test_gbtree.cc

@@ -11,7 +11,7 @@ TEST(GBTree, SelectTreeMethod) {
   size_t constexpr kCols = 10;
 
   GenericParameter generic_param;
-  generic_param.InitAllowUnknown(Args{});
+  generic_param.UpdateAllowUnknown(Args{});
   std::unique_ptr<GradientBooster> p_gbm{
     GradientBooster::Create("gbtree", &generic_param, {}, 0)};
   auto& gbtree = dynamic_cast<gbm::GBTree&> (*p_gbm);
@@ -36,7 +36,7 @@ TEST(GBTree, SelectTreeMethod) {
   ASSERT_EQ(tparam.predictor, "cpu_predictor");
 
 #ifdef XGBOOST_USE_CUDA
-  generic_param.InitAllowUnknown(Args{{"gpu_id", "0"}});
+  generic_param.UpdateAllowUnknown(Args{{"gpu_id", "0"}});
   gbtree.Configure({{"tree_method", "gpu_hist"}, {"num_feature", n_feat}});
   ASSERT_EQ(tparam.updater_seq, "grow_gpu_hist");
   ASSERT_EQ(tparam.predictor, "gpu_predictor");
@@ -64,7 +64,7 @@ TEST(GBTree, ChoosePredictor) {
   std::string n_feat = std::to_string(kCols);
   Args args {{"tree_method", "approx"}, {"num_feature", n_feat}};
   GenericParameter generic_param;
-  generic_param.InitAllowUnknown(Args{{"gpu_id", "0"}});
+  generic_param.UpdateAllowUnknown(Args{{"gpu_id", "0"}});
 
   auto& data = (*(p_mat->GetBatches<SparsePage>().begin())).data;
 

tests/cpp/objective/test_objective.cc

@@ -9,7 +9,7 @@ TEST(Objective, UnknownFunction) {
   xgboost::ObjFunction* obj = nullptr;
   xgboost::GenericParameter tparam;
   std::vector<std::pair<std::string, std::string>> args;
-  tparam.InitAllowUnknown(args);
+  tparam.UpdateAllowUnknown(args);
 
   EXPECT_ANY_THROW(obj = xgboost::ObjFunction::Create("unknown_name", &tparam));
   EXPECT_NO_THROW(obj = xgboost::ObjFunction::Create("reg:squarederror", &tparam));

tests/cpp/objective/test_ranking_obj.cc

@@ -38,7 +38,7 @@ TEST(Objective, DeclareUnifiedTest(PairwiseRankingGPair)) {
 
 TEST(Objective, DeclareUnifiedTest(NDCG_Json_IO)) {
   xgboost::GenericParameter tparam;
-  tparam.InitAllowUnknown(Args{});
+  tparam.UpdateAllowUnknown(Args{});
 
   std::unique_ptr<xgboost::ObjFunction> obj {
     xgboost::ObjFunction::Create("rank:ndcg", &tparam)

tests/cpp/test_logging.cc

@@ -53,6 +53,7 @@ TEST(Logging, Basic) {
   output = testing::internal::GetCapturedStderr();
   ASSERT_NE(output.find("Test Log Console"), std::string::npos);
 
+  args["silent"] = "False";
   args["verbosity"] = "1";  // restore
   ConsoleLogger::Configure({args.cbegin(), args.cend()});
 }

tests/cpp/tree/test_prune.cc

@@ -33,7 +33,7 @@ TEST(Updater, Prune) {
 
   // prepare tree
   RegTree tree = RegTree();
-  tree.param.InitAllowUnknown(cfg);
+  tree.param.UpdateAllowUnknown(cfg);
   std::vector<RegTree*> trees {&tree};
   // prepare pruner
   std::unique_ptr<TreeUpdater> pruner(TreeUpdater::Create("prune", &lparam));

tests/cpp/tree/test_quantile_hist.cc

@@ -262,7 +262,7 @@ class QuantileHistMock : public QuantileHistMaker {
     gmat.Init((*dmat_).get(), kMaxBins);
 
     RegTree tree = RegTree();
-    tree.param.InitAllowUnknown(cfg_);
+    tree.param.UpdateAllowUnknown(cfg_);
 
     std::vector<GradientPair> gpair =
         { {0.23f, 0.24f}, {0.23f, 0.24f}, {0.23f, 0.24f}, {0.23f, 0.24f},
@@ -273,7 +273,7 @@ class QuantileHistMock : public QuantileHistMaker {
 
   void TestBuildHist() {
     RegTree tree = RegTree();
-    tree.param.InitAllowUnknown(cfg_);
+    tree.param.UpdateAllowUnknown(cfg_);
 
     size_t constexpr kMaxBins = 4;
     common::GHistIndexMatrix gmat;
@@ -284,7 +284,7 @@ class QuantileHistMock : public QuantileHistMaker {
 
   void TestEvaluateSplit() {
     RegTree tree = RegTree();
-    tree.param.InitAllowUnknown(cfg_);
+    tree.param.UpdateAllowUnknown(cfg_);
 
     builder_->TestEvaluateSplit(gmatb_, tree);
   }

tests/cpp/tree/test_refresh.cc

@@ -28,7 +28,7 @@ TEST(Updater, Refresh) {
 
   RegTree tree = RegTree();
   auto lparam = CreateEmptyGenericParam(GPUIDX);
-  tree.param.InitAllowUnknown(cfg);
+  tree.param.UpdateAllowUnknown(cfg);
   std::vector<RegTree*> trees {&tree};
   std::unique_ptr<TreeUpdater> refresher(TreeUpdater::Create("refresh", &lparam));