From 46aa0b914ace715584f23efe950e6165aad6b671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barth=C3=A9l=C3=A9my=20von=20Haller?= Date: Tue, 21 Nov 2023 14:30:45 +0100 Subject: [PATCH] =?UTF-8?q?[QC-1020]=20Fall=20back=20to=20default=20values?= =?UTF-8?q?=20if=20beamtype=20or=20runtype=20cannot=20b=E2=80=A6=20(#2033)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [QC-1020] Fall back to default values if beamtype or runtype cannot be found. * [QC-1020] Remove cout * format * explicit use of nullopt * Address Andrea's comments --- .../include/QualityControl/CustomParameters.h | 26 +++++++-- Framework/src/CustomParameters.cxx | 35 ++++++++++-- Framework/test/testCustomParameters.cxx | 56 +++++++++++++++++++ doc/Advanced.md | 13 ++++- 4 files changed, 118 insertions(+), 12 deletions(-) diff --git a/Framework/include/QualityControl/CustomParameters.h b/Framework/include/QualityControl/CustomParameters.h index 93fb3b7124..b5599c2f96 100644 --- a/Framework/include/QualityControl/CustomParameters.h +++ b/Framework/include/QualityControl/CustomParameters.h @@ -69,16 +69,33 @@ class CustomParameters /** * Return the value for the given key, runType and beamType. + * If no key is found for the runType and the Beamtype, the fallback is to substitute with "default", first for beamType then for runType. + * An exception is raised only if the key could not be found in any combination of the provided run and beam types with "default". * @param key * @param runType * @param beamType * @return the value for the given key, runType and beamType. - * @throw std::out_of_range if no key-value pair corresponds to this key and to these beamType and runType + * @throw std::out_of_range if no key-value pair corresponds to this key and to these beamType and runType and that substitutions + * with "default" failed. */ std::string at(const std::string& key, const std::string& runType = "default", const std::string& beamType = "default") const; /** - * Return the optional value for the given key, runType and beamType (the two latter optional). + * Return the value for the given key, runType and beamType. + * If no key is found for the runType and the Beamtype, the fallback is to substitute with "default", first for beamType then for runType. + * An exception is raised only if the key could not be found in any combination of the provided run and beam types with "default". + * @param key + * @param activity + * @return the value for the given key and for the given activity. + * @throw std::out_of_range if no key-value pair corresponds to this key and to this activity and that substitutions + * with "default" failed. + */ + std::string at(const std::string& key, const Activity& activity) const; + + /** + * Return the optional value for the given key, runType and beamType. + * If no key is found for the runType and the Beamtype, the fallback is to substitute with "default", first for beamType then for runType. + * Empty is only returned if the key could not be found in any combination of the provided run and beam types with "default". * @param key * @param runType * @param beamType @@ -87,8 +104,9 @@ class CustomParameters std::optional atOptional(const std::string& key, const std::string& runType = "default", const std::string& beamType = "default") const; /** - * Return the optional value for the given key in the specified activity. - * @param key + * Return the optional value for the given key, runType and beamType. + * If no key is found for the runType and the Beamtype, the fallback is to substitute with "default", first for beamType then for runType. + * Empty is only returned if the key could not be found in any combination of the provided run and beam types with "default". * @param key * @param activity * @return an optional with the value for the given key and for the given activity. */ diff --git a/Framework/src/CustomParameters.cxx b/Framework/src/CustomParameters.cxx index 646ffc09cf..e703760ca5 100644 --- a/Framework/src/CustomParameters.cxx +++ b/Framework/src/CustomParameters.cxx @@ -12,6 +12,8 @@ #include "QualityControl/CustomParameters.h" #include #include +#include +#include namespace o2::quality_control::core { @@ -54,16 +56,39 @@ const std::unordered_map& CustomParameters::getAllDefa std::string CustomParameters::at(const std::string& key, const std::string& runType, const std::string& beamType) const { - return mCustomParameters.at(runType).at(beamType).at(key); + auto optionalResult = atOptional(key, runType, beamType); // just reuse the logic we developed in atOptional + if (!optionalResult.has_value()) { + return mCustomParameters.at(runType).at(beamType).at(key); // we know we will get a out_of_range exception + } + return optionalResult.value(); +} + +std::string CustomParameters::at(const std::string& key, const Activity& activity) const +{ + // Get the proper parameter for the given activity + const int runType = activity.mType; // get the type for this run + // convert it to a string (via a string_view as this is what we get from O2) + const std::string_view runTypeStringView = o2::parameters::GRPECS::RunTypeNames[runType]; + const std::string runTypeString{ runTypeStringView }; + // get the param + return at(key, runTypeString, activity.mBeamType); } std::optional CustomParameters::atOptional(const std::string& key, const std::string& runType, const std::string& beamType) const { - try { - return mCustomParameters.at(runType).at(beamType).at(key); - } catch (const std::out_of_range& exc) { - return {}; + std::optional result = std::nullopt; + const std::vector runTypes = { runType, std::string("default") }; + const std::vector beamTypes = { beamType, std::string("default") }; + for (const auto& rt : runTypes) { + for (const auto& bt : beamTypes) { + try { + result = mCustomParameters.at(rt).at(bt).at(key); + return result; + } catch (const std::out_of_range& exc) { // ignored on purpose + } + } } + return result; } std::optional CustomParameters::atOptional(const std::string& key, const Activity& activity) const diff --git a/Framework/test/testCustomParameters.cxx b/Framework/test/testCustomParameters.cxx index 53be5903f0..1b47892ca8 100644 --- a/Framework/test/testCustomParameters.cxx +++ b/Framework/test/testCustomParameters.cxx @@ -182,3 +182,59 @@ BOOST_AUTO_TEST_CASE(test_cp_new_access_pattern) BOOST_CHECK_EQUAL(std::stoi(param2->second), 1); } } + +BOOST_AUTO_TEST_CASE(test_default_if_not_found_at_optional) +{ + CustomParameters cp; + + // no default values are in the CP, we get an empty result + BOOST_CHECK_EQUAL(cp.atOptional("key", "PHYSICS", "proton-proton").has_value(), false); + BOOST_CHECK_EQUAL(cp.atOptional("key", "TECHNICAL", "proton-proton").has_value(), false); + + // prepare the CP + cp.set("key", "valueDefaultDefault", "default", "default"); + cp.set("key", "valuePhysicsDefault", "PHYSICS", "default"); + cp.set("key", "valuePhysicsPbPb", "PHYSICS", "Pb-Pb"); + cp.set("key", "valueCosmicsDefault", "COSMICS", "default"); + cp.set("key", "valueCosmicsDefault", "default", "PROTON-PROTON"); + + // check the data + BOOST_CHECK_EQUAL(cp.atOptional("key").value(), "valueDefaultDefault"); + BOOST_CHECK_EQUAL(cp.atOptional("key", "PHYSICS").value(), "valuePhysicsDefault"); + BOOST_CHECK_EQUAL(cp.atOptional("key", "PHYSICS", "Pb-Pb").value(), "valuePhysicsPbPb"); + BOOST_CHECK_EQUAL(cp.atOptional("key", "COSMICS", "default").value(), "valueCosmicsDefault"); + BOOST_CHECK_EQUAL(cp.atOptional("key", "default", "PROTON-PROTON").value(), "valueCosmicsDefault"); + + // check when something is missing + BOOST_CHECK_EQUAL(cp.atOptional("key", "PHYSICS", "PROTON-PROTON").value(), "valuePhysicsDefault"); // key is not defined for pp + BOOST_CHECK_EQUAL(cp.atOptional("key", "TECHNICAL", "STRANGE").value(), "valueDefaultDefault"); // key is not defined for run nor beam + BOOST_CHECK_EQUAL(cp.atOptional("key", "TECHNICAL", "PROTON-PROTON").value(), "valueCosmicsDefault"); // key is not defined for technical +} + +BOOST_AUTO_TEST_CASE(test_default_if_not_found_at) +{ + CustomParameters cp; + + // no default values are in the CP, we get an empty result + BOOST_CHECK_THROW(cp.at("key", "PHYSICS", "proton-proton"), std::out_of_range); + BOOST_CHECK_THROW(cp.at("key", "TECHNICAL", "proton-proton"), std::out_of_range); + + // prepare the CP + cp.set("key", "valueDefaultDefault", "default", "default"); + cp.set("key", "valuePhysicsDefault", "PHYSICS", "default"); + cp.set("key", "valuePhysicsPbPb", "PHYSICS", "Pb-Pb"); + cp.set("key", "valueCosmicsDefault", "COSMICS", "default"); + cp.set("key", "valueCosmicsDefault", "default", "PROTON-PROTON"); + + // check the data + BOOST_CHECK_EQUAL(cp.at("key"), "valueDefaultDefault"); + BOOST_CHECK_EQUAL(cp.at("key", "PHYSICS"), "valuePhysicsDefault"); + BOOST_CHECK_EQUAL(cp.at("key", "PHYSICS", "Pb-Pb"), "valuePhysicsPbPb"); + BOOST_CHECK_EQUAL(cp.at("key", "COSMICS", "default"), "valueCosmicsDefault"); + BOOST_CHECK_EQUAL(cp.at("key", "default", "PROTON-PROTON"), "valueCosmicsDefault"); + + // check when something is missing + BOOST_CHECK_EQUAL(cp.at("key", "PHYSICS", "PROTON-PROTON"), "valuePhysicsDefault"); // key is not defined for pp + BOOST_CHECK_EQUAL(cp.at("key", "TECHNICAL", "STRANGE"), "valueDefaultDefault"); // key is not defined for run nor beam + BOOST_CHECK_EQUAL(cp.at("key", "TECHNICAL", "PROTON-PROTON"), "valueCosmicsDefault"); // key is not defined for technical +} diff --git a/doc/Advanced.md b/doc/Advanced.md index 9b6526be3f..9f09bf0f27 100644 --- a/doc/Advanced.md +++ b/doc/Advanced.md @@ -1044,7 +1044,7 @@ The simple, deprecated, syntax is "tasks": { "QcTask": { "taskParameters": { - "myOwnKey": "myOwnValue" + "myOwnKey1": "myOwnValue1" }, ``` It is accessed with : `mCustomParameters["myOwnKey"]`. @@ -1056,7 +1056,7 @@ The new syntax is "extendedTaskParameters": { "default": { "default": { - "myOwnKey": "myOwnValue", + "myOwnKey1": "myOwnValue1", "myOwnKey2": "myOwnValue2", "myOwnKey3": "myOwnValue3" } @@ -1087,8 +1087,12 @@ The values can be accessed in various ways. ### Access optional values with or without activity +The value for the key, runType and beamType is returned if found, or an empty value otherwise. +However, before returning an empty value we try to substitute the runType and the beamType with "default". + ```c++ -// returns an Optional if it finds the key `myOwnKey` for the runType and beamType of the provided activity. +// returns an Optional if it finds the key `myOwnKey` for the runType and beamType of the provided activity, +// or if it can find the key with the runType or beamType substituted with "default". auto param = mCustomParameters.atOptional("myOwnKey", activity); // activity is "PHYSICS", "Pb-Pb" , returns "myOwnValue1d" // same but passing directly the run and beam types auto param = mCustomParameters.atOptional("myOwnKey", "PHYSICS", "Pb-Pb"); // returns "myOwnValue1d" @@ -1098,6 +1102,9 @@ auto param = mCustomParameters.atOptional("myOwnKey", "PHYSICS"); // returns "my ### Access values directly specifying the run and beam type +The value for the key, runType and beamType is returned if found, or an exception is thrown otherwise.. +However, before throwing we try to substitute the runType and the beamType with "default". + ```c++ mCustomParameters["myOwnKey"]; // considering that run and beam type are `default` --> returns `myOwnValue` mCustomParameters.at("myOwnKey"); // returns `myOwnValue`