From 1e1a766f111479227f3eae3b1e247a464dd1b7ca Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 8 May 2024 14:29:01 -0400 Subject: [PATCH] Expose parameter table verbosity, strictness This addresses https://github.com/eic/EICrecon/issues/1426 --- src/libraries/JANA/CLI/JMain.cc | 2 +- src/libraries/JANA/JApplication.cc | 11 +- .../JANA/Services/JParameterManager.cc | 128 ++++++++++++++---- .../JANA/Services/JParameterManager.h | 8 ++ .../Services/JParameterManagerTests.cc | 18 +++ 5 files changed, 135 insertions(+), 32 deletions(-) diff --git a/src/libraries/JANA/CLI/JMain.cc b/src/libraries/JANA/CLI/JMain.cc index a28d31a02..dd1eb6d1d 100644 --- a/src/libraries/JANA/CLI/JMain.cc +++ b/src/libraries/JANA/CLI/JMain.cc @@ -104,7 +104,7 @@ int Execute(JApplication* app, UserOptions &options) { if (options.flags[Benchmark]) { JBenchmarker benchmarker(app); // Show benchmarking configs only if benchmarking mode specified } - app->GetJParameterManager()->PrintParameters(true); + app->GetJParameterManager()->PrintParameters(2, 1); } else if (options.flags[DumpConfigs]) { // Load all plugins, dump parameters to file, exit without running anything diff --git a/src/libraries/JANA/JApplication.cc b/src/libraries/JANA/JApplication.cc index 9dd5d4e53..5941358bf 100644 --- a/src/libraries/JANA/JApplication.cc +++ b/src/libraries/JANA/JApplication.cc @@ -155,6 +155,13 @@ void JApplication::Initialize() { m_processing_controller = m_service_locator->get(); // Get deps from SL m_processing_controller->initialize(); + // At this point, all components should have been provided and all parameter values should have been set. + // Let's report what we found! + // Print summary of all config parameters + m_params->PrintParameters(); + + LOG_INFO(m_logger) << GetComponentSummary() << LOG_END; + m_initialized = true; // This needs to be at the end so that m_initialized==false while InitPlugin() is being called } @@ -182,10 +189,6 @@ void JApplication::Run(bool wait_until_finished) { Initialize(); if(m_quitting) return; - // Print summary of all config parameters (if any aren't default) - m_params->PrintParameters(false); - - LOG_INFO(m_logger) << GetComponentSummary() << LOG_END; LOG_INFO(m_logger) << "Starting processing with " << m_desired_nthreads << " threads requested..." << LOG_END; m_processing_controller->run(m_desired_nthreads); diff --git a/src/libraries/JANA/Services/JParameterManager.cc b/src/libraries/JANA/Services/JParameterManager.cc index 1b07574b4..4c97d894f 100644 --- a/src/libraries/JANA/Services/JParameterManager.cc +++ b/src/libraries/JANA/Services/JParameterManager.cc @@ -75,49 +75,119 @@ JParameter* JParameterManager::FindParameter(std::string name) { return result->second; } +void JParameterManager::PrintParameters() { + // In an ideal world, these parameters would be declared in Init(). However, we always have the chicken-and-egg problem to contend with + // when initializing ParameterManager and other Services. As long as PrintParameters() gets called at the end of JApplication::Initialize(), + // this is fine. + SetDefaultParameter("jana:parameter_verbosity", m_verbosity, "0: Don't show parameters table\n" + "1: Show user-provided parameters only\n" + "2: Show defaulted parameters\n" + "3: Show defaulted advanced parameters"); + + SetDefaultParameter("jana:parameter_strictness", m_strictness, "0: Ignore unused parameters\n" + "1: Warn on unused parameters\n" + "2: Throw on unused parameters"); + + if ((m_verbosity < 0) || (m_verbosity > 3)) throw JException("jana:parameter_verbosity: Bad parameter value!"); + if ((m_strictness < 0) || (m_strictness > 2)) throw JException("jana:parameter_strictness: Bad parameter value!"); + + PrintParameters(m_verbosity, m_strictness); +} + +void JParameterManager::PrintParameters(bool show_defaulted, bool show_advanced, bool warn_on_unused) { + int verbosity = 1; + int strictness = 0; + + if (show_advanced) { + verbosity = 3; + } + else if (show_defaulted) { + verbosity = 2; + } + if (warn_on_unused) { + strictness = 1; + } + PrintParameters(verbosity, strictness); +} + + /// @brief Prints parameters to stdout /// -/// @param [in] all If false, prints only parameters whose values differ from the defaults. -/// If true, prints all parameters. -/// Defaults to false. -void JParameterManager::PrintParameters(bool show_defaulted, bool show_advanced, bool warn_on_unused) { +/// @param [in] int verbosity 0: Don't show parameters table +/// 1: Show parameters with user-provided values only +/// 2: Show parameters with default values, except those marked "advanced" +/// 3: Show parameters with default values, including those marked "advanced" +/// +/// @param [in] int strictness 0: Ignore unused parameters +/// 1: Warn on unused parameters +/// 2: Throw on unused parameters +void JParameterManager::PrintParameters(int verbosity, int strictness) { - // First we filter - vector params_to_print; - bool warnings_present = false; + if (verbosity == 0) { + LOG_INFO(m_logger) << "Configuration parameters table hidden. Set jana:parameter_verbosity > 0 to view." << LOG_END; + return; + } - for (auto& p : m_parameters) { - string key = p.first; - auto j = p.second; + bool warnings_present = false; + bool strictness_violation = false; - if ((!show_defaulted) && j->IsDefault()) continue; // Hide all parameters that were set by default and the user didn't provide - if (j->IsDeprecated() && j->IsDefault()) continue; // Hide deprecated parameters that are NOT in use - if (!show_advanced && j->IsAdvanced() && j->IsDefault()) continue; + // We don't need to show "Advanced" as a warning unless we are using full verbosity + if (verbosity == 3) { + warnings_present = true; + } - if (j->IsDeprecated() && !j->IsDefault()) { - // Warn for any deprecated parameters that are overriden. - LOG_WARN(m_logger) << "Parameter '" << key << "' has been deprecated and will no longer be supported in the next release." << LOG_END; - warnings_present = true; - } - if (warn_on_unused && !j->IsUsed()) { - // Warn about any unused parameters - LOG_WARN(m_logger) << "Parameter '" << key << "' appears to be unused at this time. Possible typo?" << LOG_END; + // Check for warnings and unused parameters first + // The former might change the table columns and the latter might change the filter verbosity + for (auto& pair : m_parameters) { + const auto& key = pair.first; + auto param = pair.second; + + if ((strictness > 0) && (!param->IsDefault()) && (!param->IsUsed())) { + strictness_violation = true; warnings_present = true; + if (strictness < 2) { + LOG_WARN(m_logger) << "Parameter '" << key << "' appears to be unused. Possible typo?" << LOG_END; + } + else { + LOG_FATAL(m_logger) << "Parameter '" << key << "' appears to be unused. Possible typo?" << LOG_END; + } } - if (j->IsAdvanced()) { - // Inform user that this parameter is advanced + if ((!param->IsDefault()) && (param->IsDeprecated())) { + LOG_WARN(m_logger) << "Parameter '" << key << "' has been deprecated and will no longer be supported in the next release." << LOG_END; warnings_present = true; } - params_to_print.push_back(p.second); } + if (strictness_violation) { + LOG_WARN(m_logger) << "Printing parameter table with full verbosity due to strictness warning" << LOG_END; + verbosity = 3; // Crank up verbosity before printing out table + } + + // Filter table + vector params_to_print; + + for (auto& pair : m_parameters) { + auto param = pair.second; + + if (param->IsDeprecated() && (param->IsDefault())) continue; + // Always hide deprecated parameters that are NOT in use + + if ((verbosity == 1) && (param->IsDefault())) continue; + // At verbosity level 1, hide all default-valued parameters + + if ((verbosity == 2) && (param->IsDefault()) && (param->IsAdvanced())) continue; + // At verbosity level 2, hide only advanced default-valued parameters + + params_to_print.push_back(param); + } + // If all params are set to default values, then print a one line summary and return if (params_to_print.empty()) { LOG_INFO(m_logger) << "All configuration parameters set to default values." << LOG_END; return; } - // Generate the whole table + // Print table JTablePrinter table; table.AddColumn("Name"); if (warnings_present) { @@ -134,7 +204,7 @@ void JParameterManager::PrintParameters(bool show_defaulted, bool show_advanced, // If deprecated, it no longer matters whether it is advanced or not. If unused, won't show up here anyway. warning = "Deprecated"; } - else if (warn_on_unused && !p->IsUsed()) { + else if (!p->IsUsed()) { // Can't be both deprecated and unused, since JANA only finds out that it is deprecated by trying to use it // Can't be both advanced and unused, since JANA only finds out that it is advanced by trying to use it warning = "Unused"; @@ -143,7 +213,6 @@ void JParameterManager::PrintParameters(bool show_defaulted, bool show_advanced, warning = "Advanced"; } - table | p->GetKey() | warning | p->GetValue() @@ -160,6 +229,11 @@ void JParameterManager::PrintParameters(bool show_defaulted, bool show_advanced, std::ostringstream ss; table.Render(ss); LOG_INFO(m_logger) << "Configuration Parameters\n" << ss.str() << LOG_END; + + // Now that we've printed the table, we can throw an exception if we are being super strict + if (strictness_violation && strictness > 1) { + throw JException("Unrecognized parameters found! Throwing an exception because jana:parameter_strictness=2. See full parameters table in log."); + } } /// @brief Access entire map of parameters diff --git a/src/libraries/JANA/Services/JParameterManager.h b/src/libraries/JANA/Services/JParameterManager.h index 7839db79f..9bcd3fd91 100644 --- a/src/libraries/JANA/Services/JParameterManager.h +++ b/src/libraries/JANA/Services/JParameterManager.h @@ -84,6 +84,11 @@ class JParameterManager : public JService { JParameter* FindParameter(std::string); + void PrintParameters(); + + void PrintParameters(int verbosity, int strictness); + + [[deprecated]] void PrintParameters(bool show_defaulted, bool show_advanced=true, bool warn_on_unused=false); std::map GetAllParameters(); @@ -141,6 +146,9 @@ class JParameterManager : public JService { std::map m_parameters; + int m_strictness = 1; + int m_verbosity = 1; + JLogger m_logger; std::mutex m_mutex; diff --git a/src/programs/unit_tests/Services/JParameterManagerTests.cc b/src/programs/unit_tests/Services/JParameterManagerTests.cc index fbc2c6c30..8c75bf0a1 100644 --- a/src/programs/unit_tests/Services/JParameterManagerTests.cc +++ b/src/programs/unit_tests/Services/JParameterManagerTests.cc @@ -416,6 +416,24 @@ TEST_CASE("JParameterManager_CompileTimeErrorForParseAndStringify") { } +TEST_CASE("JParameterManager_Strictness") { + JParameterManager sut; + sut.SetParameter("jana:parameter_strictness", 2); + sut.SetParameter("jana:unused", 22); + bool exception_found = false; + try { + sut.PrintParameters(); + } + catch (JException& e) { + exception_found = true; + } + REQUIRE(exception_found == true); +} + + + + +