From ad4a1c732cfad6d6dc5be012145a2dac37413cc7 Mon Sep 17 00:00:00 2001 From: Jiaming Yuan Date: Wed, 11 Dec 2019 19:49:01 +0800 Subject: [PATCH] Small refinements for JSON model. (#5112) * Naming consistency. * Remove duplicated test. --- src/gbm/gbtree_model.cc | 4 ++-- src/learner.cc | 6 +++--- tests/cpp/gbm/test_gblinear.cc | 12 ++---------- tests/cpp/gbm/test_gbtree.cc | 16 +++++++--------- tests/cpp/test_learner.cc | 6 +++--- tests/cpp/tree/test_gpu_hist.cu | 1 - tests/python/test_basic_models.py | 4 ++-- 7 files changed, 19 insertions(+), 30 deletions(-) diff --git a/src/gbm/gbtree_model.cc b/src/gbm/gbtree_model.cc index ccbdcd8cf..f4125880e 100644 --- a/src/gbm/gbtree_model.cc +++ b/src/gbm/gbtree_model.cc @@ -40,7 +40,7 @@ void GBTreeModel::Load(dmlc::Stream* fi) { void GBTreeModel::SaveModel(Json* p_out) const { auto& out = *p_out; CHECK_EQ(param.num_trees, static_cast(trees.size())); - out["model_param"] = toJson(param); + out["gbtree_model_param"] = toJson(param); std::vector trees_json; size_t t = 0; for (auto const& tree : trees) { @@ -61,7 +61,7 @@ void GBTreeModel::SaveModel(Json* p_out) const { } void GBTreeModel::LoadModel(Json const& in) { - fromJson(in["model_param"], ¶m); + fromJson(in["gbtree_model_param"], ¶m); trees.clear(); trees_to_update.clear(); diff --git a/src/learner.cc b/src/learner.cc index a57a57796..2386914c5 100644 --- a/src/learner.cc +++ b/src/learner.cc @@ -269,7 +269,7 @@ class LearnerImpl : public Learner { void LoadModel(Json const& in) override { CHECK(IsA(in)); Version::Load(in, false); - auto const& learner = get(in["Learner"]); + auto const& learner = get(in["learner"]); mparam_.FromJson(learner.at("learner_model_param")); auto const& objective_fn = learner.at("objective"); @@ -305,8 +305,8 @@ class LearnerImpl : public Learner { Version::Save(p_out); Json& out { *p_out }; - out["Learner"] = Object(); - auto& learner = out["Learner"]; + out["learner"] = Object(); + auto& learner = out["learner"]; learner["learner_model_param"] = mparam_.ToJson(); learner["gradient_booster"] = Object(); diff --git a/tests/cpp/gbm/test_gblinear.cc b/tests/cpp/gbm/test_gblinear.cc index a63040d9d..dbd7885e3 100644 --- a/tests/cpp/gbm/test_gblinear.cc +++ b/tests/cpp/gbm/test_gblinear.cc @@ -35,22 +35,14 @@ TEST(GBLinear, Json_IO) { std::string model_str; Json::Dump(model, &model_str); - model = Json::Load({model_str.c_str(), model_str.size()}); + model = Json::Load(StringView{model_str.c_str(), model_str.size()}); ASSERT_TRUE(IsA(model)); - model = model["model"]; { + model = model["model"]; auto weights = get(model["weights"]); ASSERT_EQ(weights.size(), 17); } - - { - model = Json::Load({model_str.c_str(), model_str.size()}); - model = model["model"]; - auto weights = get(model["weights"]); - ASSERT_EQ(weights.size(), 17); // 16 + 1 (bias) - } - } } // namespace gbm diff --git a/tests/cpp/gbm/test_gbtree.cc b/tests/cpp/gbm/test_gbtree.cc index 3cb6d0e29..f0a77b789 100644 --- a/tests/cpp/gbm/test_gbtree.cc +++ b/tests/cpp/gbm/test_gbtree.cc @@ -64,7 +64,7 @@ TEST(GBTree, ChoosePredictor) { } ASSERT_TRUE(data.HostCanWrite()); dmlc::TemporaryDirectory tempdir; - const std::string fname = tempdir.path + "/model_para.bst"; + const std::string fname = tempdir.path + "/model_param.bst"; { std::unique_ptr fo(dmlc::Stream::Create(fname.c_str(), "w")); @@ -117,17 +117,15 @@ TEST(GBTree, Json_IO) { CreateTrainedGBM("gbtree", Args{}, kRows, kCols, &mparam, &gparam) }; Json model {Object()}; - model["model"] = Object(); - auto& j_model = model["model"]; - gbm->SaveModel(&j_model); + gbm->SaveModel(&model); - std::stringstream ss; - Json::Dump(model, &ss); + std::string model_str; + Json::Dump(model, &model_str); - auto model_str = ss.str(); - model = Json::Load({model_str.c_str(), model_str.size()}); - ASSERT_EQ(get(model["model"]["name"]), "gbtree"); + auto loaded_model = Json::Load(StringView{model_str.c_str(), model_str.size()}); + ASSERT_EQ(get(loaded_model["name"]), "gbtree"); + ASSERT_TRUE(IsA(loaded_model["model"]["gbtree_model_param"])); } TEST(Dart, Json_IO) { diff --git a/tests/cpp/test_learner.cc b/tests/cpp/test_learner.cc index 3c7ed5450..6e81ca195 100644 --- a/tests/cpp/test_learner.cc +++ b/tests/cpp/test_learner.cc @@ -143,7 +143,7 @@ TEST(Learner, Json_ModelIO) { for (int32_t iter = 0; iter < kIters; ++iter) { learner->UpdateOneIter(iter, p_dmat.get()); } - learner->SetAttr("bset_score", "15.2"); + learner->SetAttr("best_score", "15.2"); Json out { Object() }; learner->SaveModel(&out); @@ -153,8 +153,8 @@ TEST(Learner, Json_ModelIO) { learner->Configure(); learner->SaveModel(&new_in); - ASSERT_TRUE(IsA(out["Learner"]["attributes"])); - ASSERT_EQ(get(out["Learner"]["attributes"]).size(), 1); + ASSERT_TRUE(IsA(out["learner"]["attributes"])); + ASSERT_EQ(get(out["learner"]["attributes"]).size(), 1); ASSERT_EQ(out, new_in); } diff --git a/tests/cpp/tree/test_gpu_hist.cu b/tests/cpp/tree/test_gpu_hist.cu index 60a8dd886..2ba1c9b19 100644 --- a/tests/cpp/tree/test_gpu_hist.cu +++ b/tests/cpp/tree/test_gpu_hist.cu @@ -13,7 +13,6 @@ #include "xgboost/json.h" #include "../../../src/data/sparse_page_source.h" -#include "../../../src/gbm/gbtree_model.h" #include "../../../src/tree/updater_gpu_hist.cu" #include "../../../src/tree/updater_gpu_common.cuh" #include "../../../src/common/common.h" diff --git a/tests/python/test_basic_models.py b/tests/python/test_basic_models.py index 09df895e5..eb6b1a093 100644 --- a/tests/python/test_basic_models.py +++ b/tests/python/test_basic_models.py @@ -213,12 +213,12 @@ class TestModels(unittest.TestCase): with open('./model.json', 'r') as fd: j_model = json.load(fd) - assert isinstance(j_model['Learner'], dict) + assert isinstance(j_model['learner'], dict) bst = xgb.Booster(model_file='./model.json') with open('./model.json', 'r') as fd: j_model = json.load(fd) - assert isinstance(j_model['Learner'], dict) + assert isinstance(j_model['learner'], dict) os.remove('model.json')