From 46427128bf92af4bf8f6f8914cca2a87c3caada0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Morales?= Date: Mon, 10 Oct 2022 22:53:55 -0500 Subject: [PATCH] suppress alias warnings with verbosity<0 (fixes #4518) (#5253) --- include/LightGBM/config.h | 4 +- src/application/application.cpp | 23 +++++---- src/io/config.cpp | 65 ++++++++++++++++-------- tests/python_package_test/test_engine.py | 42 +++++++++++++++ 4 files changed, 103 insertions(+), 31 deletions(-) diff --git a/include/LightGBM/config.h b/include/LightGBM/config.h index e88c4d7b70b7..f4791394c43c 100644 --- a/include/LightGBM/config.h +++ b/include/LightGBM/config.h @@ -86,7 +86,9 @@ struct Config { */ inline static bool SortAlias(const std::string& x, const std::string& y); - static void KV2Map(std::unordered_map* params, const char* kv); + static void KeepFirstValues(const std::unordered_map>& params, std::unordered_map* out); + static void KV2Map(std::unordered_map>* params, const char* kv); + static void SetVerbosity(const std::unordered_map>& params); static std::unordered_map Str2Map(const char* parameters); #ifndef __NVCC__ diff --git a/src/application/application.cpp b/src/application/application.cpp index 7f7fc816cf4b..d69efb5017ae 100644 --- a/src/application/application.cpp +++ b/src/application/application.cpp @@ -48,15 +48,15 @@ Application::~Application() { } void Application::LoadParameters(int argc, char** argv) { + std::unordered_map> all_params; std::unordered_map params; for (int i = 1; i < argc; ++i) { - Config::KV2Map(¶ms, argv[i]); + Config::KV2Map(&all_params, argv[i]); } - // check for alias - ParameterAlias::KeyAliasTransform(¶ms); // read parameters from config file - if (params.count("config") > 0) { - TextReader config_reader(params["config"].c_str(), false); + bool config_file_ok = true; + if (all_params.count("config") > 0) { + TextReader config_reader(all_params["config"][0].c_str(), false); config_reader.ReadAllLines(); if (!config_reader.Lines().empty()) { for (auto& line : config_reader.Lines()) { @@ -68,16 +68,19 @@ void Application::LoadParameters(int argc, char** argv) { if (line.size() == 0) { continue; } - Config::KV2Map(¶ms, line.c_str()); + Config::KV2Map(&all_params, line.c_str()); } } else { - Log::Warning("Config file %s doesn't exist, will ignore", - params["config"].c_str()); + config_file_ok = false; } } - // check for alias again + Config::SetVerbosity(all_params); + // de-duplicate params + Config::KeepFirstValues(all_params, ¶ms); + if (!config_file_ok) { + Log::Warning("Config file %s doesn't exist, will ignore", params["config"].c_str()); + } ParameterAlias::KeyAliasTransform(¶ms); - // load configs config_.Set(params); Log::Info("Finished loading parameters"); } diff --git a/src/io/config.cpp b/src/io/config.cpp index 90232045f488..72006eb50cdb 100644 --- a/src/io/config.cpp +++ b/src/io/config.cpp @@ -13,7 +13,7 @@ namespace LightGBM { -void Config::KV2Map(std::unordered_map* params, const char* kv) { +void Config::KV2Map(std::unordered_map>* params, const char* kv) { std::vector tmp_strs = Common::Split(kv, '='); if (tmp_strs.size() == 2 || tmp_strs.size() == 1) { std::string key = Common::RemoveQuotationSymbol(Common::Trim(tmp_strs[0])); @@ -22,26 +22,61 @@ void Config::KV2Map(std::unordered_map* params, const value = Common::RemoveQuotationSymbol(Common::Trim(tmp_strs[1])); } if (key.size() > 0) { - auto value_search = params->find(key); - if (value_search == params->end()) { // not set - params->emplace(key, value); - } else { - Log::Warning("%s is set=%s, %s=%s will be ignored. Current value: %s=%s", - key.c_str(), value_search->second.c_str(), key.c_str(), value.c_str(), - key.c_str(), value_search->second.c_str()); - } + params->operator[](key).emplace_back(value); } } else { Log::Warning("Unknown parameter %s", kv); } } +void GetFirstValueAsInt(const std::unordered_map>& params, std::string key, int* out) { + const auto pair = params.find(key); + if (pair != params.end()) { + auto candidate = pair->second[0].c_str(); + if (!Common::AtoiAndCheck(candidate, out)) { + Log::Fatal("Parameter %s should be of type int, got \"%s\"", key.c_str(), candidate); + } + } +} + +void Config::SetVerbosity(const std::unordered_map>& params) { + int verbosity = Config().verbosity; + GetFirstValueAsInt(params, "verbose", &verbosity); + GetFirstValueAsInt(params, "verbosity", &verbosity); + if (verbosity < 0) { + LightGBM::Log::ResetLogLevel(LightGBM::LogLevel::Fatal); + } else if (verbosity == 0) { + LightGBM::Log::ResetLogLevel(LightGBM::LogLevel::Warning); + } else if (verbosity == 1) { + LightGBM::Log::ResetLogLevel(LightGBM::LogLevel::Info); + } else { + LightGBM::Log::ResetLogLevel(LightGBM::LogLevel::Debug); + } +} + +void Config::KeepFirstValues(const std::unordered_map>& params, std::unordered_map* out) { + for (auto pair = params.begin(); pair != params.end(); ++pair) { + auto name = pair->first.c_str(); + auto values = pair->second; + out->emplace(name, values[0]); + for (size_t i = 1; i < pair->second.size(); ++i) { + Log::Warning("%s is set=%s, %s=%s will be ignored. Current value: %s=%s", + name, values[0].c_str(), + name, values[i].c_str(), + name, values[0].c_str()); + } + } +} + std::unordered_map Config::Str2Map(const char* parameters) { + std::unordered_map> all_params; std::unordered_map params; auto args = Common::Split(parameters, " \t\n\r"); for (auto arg : args) { - KV2Map(¶ms, Common::Trim(arg).c_str()); + KV2Map(&all_params, Common::Trim(arg).c_str()); } + SetVerbosity(all_params); + KeepFirstValues(all_params, ¶ms); ParameterAlias::KeyAliasTransform(¶ms); return params; } @@ -240,16 +275,6 @@ void Config::Set(const std::unordered_map& params) { save_binary = true; } - if (verbosity == 1) { - LightGBM::Log::ResetLogLevel(LightGBM::LogLevel::Info); - } else if (verbosity == 0) { - LightGBM::Log::ResetLogLevel(LightGBM::LogLevel::Warning); - } else if (verbosity >= 2) { - LightGBM::Log::ResetLogLevel(LightGBM::LogLevel::Debug); - } else { - LightGBM::Log::ResetLogLevel(LightGBM::LogLevel::Fatal); - } - // check for conflicts CheckParamConflict(); } diff --git a/tests/python_package_test/test_engine.py b/tests/python_package_test/test_engine.py index a523f9c41add..35b5113be8a5 100644 --- a/tests/python_package_test/test_engine.py +++ b/tests/python_package_test/test_engine.py @@ -6,6 +6,7 @@ import pickle import platform import random +import re from os import getenv from pathlib import Path @@ -3768,6 +3769,47 @@ def test_cegb_split_buffer_clean(): assert rmse < 10.0 +def test_verbosity_and_verbose(capsys): + X, y = make_synthetic_regression() + ds = lgb.Dataset(X, y) + params = { + 'num_leaves': 3, + 'verbose': 1, + 'verbosity': 0, + } + lgb.train(params, ds, num_boost_round=1) + expected_msg = ( + '[LightGBM] [Warning] verbosity is set=0, verbose=1 will be ignored. ' + 'Current value: verbosity=0' + ) + stdout = capsys.readouterr().out + assert expected_msg in stdout + + +@pytest.mark.parametrize('verbosity_param', lgb.basic._ConfigAliases.get("verbosity")) +@pytest.mark.parametrize('verbosity', [-1, 0]) +def test_verbosity_can_suppress_alias_warnings(capsys, verbosity_param, verbosity): + X, y = make_synthetic_regression() + ds = lgb.Dataset(X, y) + params = { + 'num_leaves': 3, + 'subsample': 0.75, + 'bagging_fraction': 0.8, + 'force_col_wise': True, + verbosity_param: verbosity, + } + lgb.train(params, ds, num_boost_round=1) + expected_msg = ( + '[LightGBM] [Warning] bagging_fraction is set=0.8, subsample=0.75 will be ignored. ' + 'Current value: bagging_fraction=0.8' + ) + stdout = capsys.readouterr().out + if verbosity >= 0: + assert expected_msg in stdout + else: + assert re.search(r'\[LightGBM\]', stdout) is None + + @pytest.mark.skipif(not PANDAS_INSTALLED, reason='pandas is not installed') def test_validate_features(): X, y = make_synthetic_regression()