From 64661232bfd553173d65359c3881b9bffed50aa0 Mon Sep 17 00:00:00 2001 From: Barthelemy Date: Tue, 14 Nov 2023 15:36:55 +0100 Subject: [PATCH 1/5] [QC-1020] Fall back to default values if beamtype or runtype cannot be found. --- .../include/QualityControl/CustomParameters.h | 14 +++-- Framework/src/CustomParameters.cxx | 25 ++++++-- Framework/test/testCustomParameters.cxx | 57 +++++++++++++++++++ doc/Advanced.md | 9 ++- 4 files changed, 95 insertions(+), 10 deletions(-) diff --git a/Framework/include/QualityControl/CustomParameters.h b/Framework/include/QualityControl/CustomParameters.h index 93fb3b7124..8390310c8f 100644 --- a/Framework/include/QualityControl/CustomParameters.h +++ b/Framework/include/QualityControl/CustomParameters.h @@ -69,16 +69,21 @@ 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 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 +92,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..9047f10b43 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,29 @@ 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::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::vector runTypes = {runType, std::string("default")}; + std::vector beamTypes = {beamType, std::string("default")}; + for(int rt = 0 ; rt < 2 ; rt++) { + for(int bt = 0 ; bt < 2 ; bt++) { + try { + std::cout << runTypes[rt] << " " << beamTypes[bt] << std::endl; + result = mCustomParameters.at(runTypes[rt]).at(beamTypes[bt]).at(key); + return result; + } catch (const std::out_of_range& exc) { + } + } } + 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..ca18e6b122 100644 --- a/Framework/test/testCustomParameters.cxx +++ b/Framework/test/testCustomParameters.cxx @@ -182,3 +182,60 @@ 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 749af51751..92ea39c9dc 100644 --- a/doc/Advanced.md +++ b/doc/Advanced.md @@ -1086,8 +1086,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" @@ -1097,6 +1101,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` From 54cb7d9a48a7c0e462455bd1ae009a4626a7fbb4 Mon Sep 17 00:00:00 2001 From: Barthelemy Date: Tue, 14 Nov 2023 15:38:54 +0100 Subject: [PATCH 2/5] [QC-1020] Remove cout --- Framework/src/CustomParameters.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/Framework/src/CustomParameters.cxx b/Framework/src/CustomParameters.cxx index 9047f10b43..61d03e0256 100644 --- a/Framework/src/CustomParameters.cxx +++ b/Framework/src/CustomParameters.cxx @@ -71,7 +71,6 @@ std::optional CustomParameters::atOptional(const std::string& key, for(int rt = 0 ; rt < 2 ; rt++) { for(int bt = 0 ; bt < 2 ; bt++) { try { - std::cout << runTypes[rt] << " " << beamTypes[bt] << std::endl; result = mCustomParameters.at(runTypes[rt]).at(beamTypes[bt]).at(key); return result; } catch (const std::out_of_range& exc) { From 91dd97bb3469c993fca2757ec03ee44cfd285d61 Mon Sep 17 00:00:00 2001 From: Barthelemy Date: Tue, 14 Nov 2023 15:39:22 +0100 Subject: [PATCH 3/5] format --- Framework/src/CustomParameters.cxx | 10 +++++----- Framework/test/testCustomParameters.cxx | 9 ++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/Framework/src/CustomParameters.cxx b/Framework/src/CustomParameters.cxx index 61d03e0256..5a53f747f5 100644 --- a/Framework/src/CustomParameters.cxx +++ b/Framework/src/CustomParameters.cxx @@ -57,7 +57,7 @@ const std::unordered_map& CustomParameters::getAllDefa std::string CustomParameters::at(const std::string& key, const std::string& runType, const std::string& beamType) const { auto optionalResult = atOptional(key, runType, beamType); // just reuse the logic we developed in atOptional - if(!optionalResult.has_value()) { + 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(); @@ -66,10 +66,10 @@ std::string CustomParameters::at(const std::string& key, const std::string& runT std::optional CustomParameters::atOptional(const std::string& key, const std::string& runType, const std::string& beamType) const { std::optional result = {}; - std::vector runTypes = {runType, std::string("default")}; - std::vector beamTypes = {beamType, std::string("default")}; - for(int rt = 0 ; rt < 2 ; rt++) { - for(int bt = 0 ; bt < 2 ; bt++) { + std::vector runTypes = { runType, std::string("default") }; + std::vector beamTypes = { beamType, std::string("default") }; + for (int rt = 0; rt < 2; rt++) { + for (int bt = 0; bt < 2; bt++) { try { result = mCustomParameters.at(runTypes[rt]).at(beamTypes[bt]).at(key); return result; diff --git a/Framework/test/testCustomParameters.cxx b/Framework/test/testCustomParameters.cxx index ca18e6b122..1b47892ca8 100644 --- a/Framework/test/testCustomParameters.cxx +++ b/Framework/test/testCustomParameters.cxx @@ -183,7 +183,6 @@ BOOST_AUTO_TEST_CASE(test_cp_new_access_pattern) } } - BOOST_AUTO_TEST_CASE(test_default_if_not_found_at_optional) { CustomParameters cp; @@ -207,8 +206,8 @@ BOOST_AUTO_TEST_CASE(test_default_if_not_found_at_optional) 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", "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 } @@ -235,7 +234,7 @@ BOOST_AUTO_TEST_CASE(test_default_if_not_found_at) 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", "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 } From a4c1bed119cfae59612c2682f111d507f8e8152d Mon Sep 17 00:00:00 2001 From: Barthelemy Date: Tue, 21 Nov 2023 08:35:57 +0100 Subject: [PATCH 4/5] explicit use of nullopt --- Framework/src/CustomParameters.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Framework/src/CustomParameters.cxx b/Framework/src/CustomParameters.cxx index 5a53f747f5..f60bb8222a 100644 --- a/Framework/src/CustomParameters.cxx +++ b/Framework/src/CustomParameters.cxx @@ -65,7 +65,7 @@ std::string CustomParameters::at(const std::string& key, const std::string& runT std::optional CustomParameters::atOptional(const std::string& key, const std::string& runType, const std::string& beamType) const { - std::optional result = {}; + std::optional result = std::nullopt; std::vector runTypes = { runType, std::string("default") }; std::vector beamTypes = { beamType, std::string("default") }; for (int rt = 0; rt < 2; rt++) { From da7919acf7a400175dcc6de12bce6325777c2ee6 Mon Sep 17 00:00:00 2001 From: Barthelemy Date: Tue, 21 Nov 2023 10:56:46 +0100 Subject: [PATCH 5/5] Address Andrea's comments --- .../include/QualityControl/CustomParameters.h | 12 ++++++++++ Framework/src/CustomParameters.cxx | 23 ++++++++++++++----- doc/Advanced.md | 4 ++-- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/Framework/include/QualityControl/CustomParameters.h b/Framework/include/QualityControl/CustomParameters.h index 8390310c8f..b5599c2f96 100644 --- a/Framework/include/QualityControl/CustomParameters.h +++ b/Framework/include/QualityControl/CustomParameters.h @@ -80,6 +80,18 @@ class CustomParameters */ std::string at(const std::string& key, const std::string& runType = "default", const std::string& beamType = "default") const; + /** + * 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. diff --git a/Framework/src/CustomParameters.cxx b/Framework/src/CustomParameters.cxx index f60bb8222a..e703760ca5 100644 --- a/Framework/src/CustomParameters.cxx +++ b/Framework/src/CustomParameters.cxx @@ -63,17 +63,28 @@ std::string CustomParameters::at(const std::string& key, const std::string& runT 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 { std::optional result = std::nullopt; - std::vector runTypes = { runType, std::string("default") }; - std::vector beamTypes = { beamType, std::string("default") }; - for (int rt = 0; rt < 2; rt++) { - for (int bt = 0; bt < 2; bt++) { + 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(runTypes[rt]).at(beamTypes[bt]).at(key); + result = mCustomParameters.at(rt).at(bt).at(key); return result; - } catch (const std::out_of_range& exc) { + } catch (const std::out_of_range& exc) { // ignored on purpose } } } diff --git a/doc/Advanced.md b/doc/Advanced.md index 92ea39c9dc..5d4255a1c7 100644 --- a/doc/Advanced.md +++ b/doc/Advanced.md @@ -1043,7 +1043,7 @@ The simple, deprecated, syntax is "tasks": { "QcTask": { "taskParameters": { - "myOwnKey": "myOwnValue" + "myOwnKey1": "myOwnValue1" }, ``` It is accessed with : `mCustomParameters["myOwnKey"]`. @@ -1055,7 +1055,7 @@ The new syntax is "extendedTaskParameters": { "default": { "default": { - "myOwnKey": "myOwnValue", + "myOwnKey1": "myOwnValue1", "myOwnKey2": "myOwnValue2", "myOwnKey3": "myOwnValue3" }