Revert "[QC-1079] Run type as a string (#2077)" (#2275)

This reverts commit 7191a3e.
AliceO2Group · May 3, 2024 · 4e96453 · 4e96453
1 parent 48f6b22
commit 4e96453
Show file tree

Hide file tree

Showing 143 changed files with 254 additions and 394 deletions.
diff --git a/Framework/CMakeLists.txt b/Framework/CMakeLists.txt
@@ -1,5 +1,5 @@
 # Produce the final Version.h using template Version.h.in and substituting
-# variables. We don't want to pollute our source tree with it, thus putting it in
+# variables. We don't want to polute our source tree with it, thus putting it in
 # binary tree.
 configure_file("include/QualityControl/Version.h.in"
  "${CMAKE_CURRENT_BINARY_DIR}/include/QualityControl/Version.h"
@@ -280,9 +280,7 @@ set(TEST_SRCS
  test/testRepoPathUtils.cxx
  test/testQualitiesToFlagCollectionConverter.cxx
  test/testUserCodeInterface.cxx
- test/testStringUtils.cxx
- test/testRunnerUtils.cxx
-)
+ )
 
 set(TEST_ARGS
  ""
@@ -302,8 +300,6 @@ set(TEST_ARGS
  ""
  ""
  ""
- ""
- ""
  )
 
 list(LENGTH TEST_SRCS count)

diff --git a/Framework/advanced-aggregator.json b/Framework/advanced-aggregator.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/advanced-external-histo.json b/Framework/advanced-external-histo.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/advanced.json b/Framework/advanced.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/basic-aggregator.json b/Framework/basic-aggregator.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/basic-external-histo.json b/Framework/basic-external-histo.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/basic-functional.json.in b/Framework/basic-functional.json.in
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE",
+ "type": "2",
  "start": "8000000",
  "end": "9000000",
  "periodName": "LHC9000x",

diff --git a/Framework/basic-no-sampling.json b/Framework/basic-no-sampling.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/basic.json b/Framework/basic.json
@@ -11,7 +11,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE",
+ "type": "2",
  "periodName": "", "": "Period name - e.g. LHC22c, LHC22c1b_test",
  "passName": "", "": "Pass type - e.g. spass, cpass1",
  "provenance": "qc", "": "Provenance - qc or qc_mc depending whether it is normal data or monte carlo data"

diff --git a/Framework/batch-test.json.in b/Framework/batch-test.json.in
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE",
+ "type": "2",
  "start": "8000000",
  "end": "9000000",
  "periodName": "LHC9000x",

diff --git a/Framework/example-default.json b/Framework/example-default.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/include/QualityControl/Activity.h b/Framework/include/QualityControl/Activity.h
@@ -32,7 +32,7 @@ class Activity
  public:
  Activity() = default;
  Activity(int id,
- const std::string& type,
+ int type,
  const std::string& periodName = "",
  const std::string& passName = "",
  const std::string& provenance = "qc",
@@ -72,7 +72,7 @@ class Activity
  virtual ~Activity() = default;
 
  int mId{ 0 };
- std::string mType{ "NONE" };
+ int mType{ 0 };
  std::string mPeriodName{};
  std::string mPassName{};
  std::string mProvenance{ "qc" };

diff --git a/Framework/include/QualityControl/CommonSpec.h b/Framework/include/QualityControl/CommonSpec.h
@@ -29,7 +29,7 @@ struct CommonSpec {
 
  std::unordered_map<std::string, std::string> database;
  int activityNumber{};
- std::string activityType = "NONE";
+ int activityType{};
  std::string activityPeriodName;
  std::string activityPassName;
  std::string activityProvenance = "qc";

diff --git a/Framework/include/QualityControl/runnerUtils.h b/Framework/include/QualityControl/runnerUtils.h
@@ -17,11 +17,9 @@
 #ifndef QUALITYCONTROL_RUNNERUTILS_H
 #define QUALITYCONTROL_RUNNERUTILS_H
 
-#include <boost/lexical_cast/bad_lexical_cast.hpp>
 #include <string>
 #include <Framework/ServiceRegistryRef.h>
 #include <Framework/RawDeviceService.h>
-#include <ProgOptions.h>
 #include <fairmq/Device.h>
 #include "QualityControl/QcInfoLogger.h"
 #include "QualityControl/Activity.h"
@@ -40,8 +38,8 @@ std::string getFirstTaskName(const std::string& configurationSource);
 std::string getFirstCheckName(const std::string& configurationSource);
 bool hasChecks(const std::string& configSource);
 
-template <typename T, typename = typename std::enable_if<std::is_arithmetic<T>::value, T>::type>
-T computeNumericalActivityField(framework::ServiceRegistryRef services, const std::string& name, T fallbackNumber = 0)
+template <typename T> // TODO we should probably limit T to numbers somehow
+T computeActivityField(framework::ServiceRegistryRef services, const std::string& name, T fallbackNumber = 0)
 {
  T result = 0;
 
@@ -61,7 +59,6 @@ T computeNumericalActivityField(framework::ServiceRegistryRef services, const st
  return result;
 }
 
-std::string_view translateIntegerRunType(const std::string& runType);
 Activity computeActivity(framework::ServiceRegistryRef services, const Activity& fallbackActivity);
 
 std::string indentTree(int level);

diff --git a/Framework/include/QualityControl/stringUtils.h b/Framework/include/QualityControl/stringUtils.h
@@ -40,11 +40,6 @@ bool decodeBool(const std::string& value);
 /// @throw std::runtime_error the value is not a bool
 bool parseBoolParam(const CustomParameters& customParameters, const std::string& name, const std::string& runType = "default", const std::string& beamType = "default");
 
-/**
- * Check if the string contains only digits.
- */
-bool isUnsignedInteger(const std::string& s);
-
 } // namespace o2::quality_control::core
 
 #endif // QC_STRING_UTILS_H
diff --git a/Framework/multiNode.json b/Framework/multiNode.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/multinode-test.json.in b/Framework/multinode-test.json.in
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE",
+ "type": "2",
  "start": "8000000",
  "end": "9000000"
  },

diff --git a/Framework/postprocessing.json b/Framework/postprocessing.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/readout-no-sampling.json b/Framework/readout-no-sampling.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/readout.json b/Framework/readout.json
@@ -10,7 +10,7 @@
  },
  "Activity": {
  "number": "42",
- "type": "NONE"
+ "type": "2"
  },
  "monitoring": {
  "url": "infologger:///debug?qc"

diff --git a/Framework/src/Activity.cxx b/Framework/src/Activity.cxx
@@ -46,7 +46,7 @@ bool Activity::matches(const Activity& other) const
  // is included in this validity. If checked for any overlaps, we would match with all past Activities, which is not
  // what we want e.g. in Post-processing triggers. Once we indicate the correct validity, we can change this behaviour.
  return (mId == 0 || mId == other.mId) &&
- (mType == "NONE" || mType == other.mType) &&
+ (mType == 0 || mType == other.mType) &&
  (mPeriodName.empty() || mPeriodName == other.mPeriodName) &&
  (mPassName.empty() || mPassName == other.mPassName) &&
  (mProvenance == other.mProvenance) && // provenance has to match!

diff --git a/Framework/src/ActivityHelpers.cxx b/Framework/src/ActivityHelpers.cxx
@@ -20,10 +20,6 @@
 #include <CCDB/BasicCCDBManager.h>
 #include "QualityControl/ObjectMetadataKeys.h"
 
-#include <DataFormatsParameters/ECSDataAdapters.h>
-#include <QualityControl/stringUtils.h>
-#include <QualityControl/runnerUtils.h>
-
 using namespace o2::quality_control::repository;
 
 namespace o2::quality_control::core::activity_helpers
@@ -32,8 +28,10 @@ namespace o2::quality_control::core::activity_helpers
 std::map<std::string, std::string> asDatabaseMetadata(const core::Activity& activity, bool putDefault)
 {
  std::map<std::string, std::string> metadata;
- if (putDefault || activity.mType != "NONE") {
- metadata[metadata_keys::runType] = activity.mType;
+ if (putDefault || activity.mType != 0) {
+ // TODO should we really treat 0 as none?
+ // we could consider making Activity use std::optional to be clear about this
+ metadata[metadata_keys::runType] = std::to_string(activity.mType);
  }
  if (putDefault || activity.mId != 0) {
  metadata[metadata_keys::runNumber] = std::to_string(activity.mId);
@@ -51,12 +49,7 @@ core::Activity asActivity(const std::map<std::string, std::string>& metadata, co
 {
  core::Activity activity;
  if (auto runType = metadata.find(metadata_keys::runType); runType != metadata.end()) {
- if (isUnsignedInteger(runType->second)) {
- // we probably got the former representation of run types, i.e. an integer. We convert it as best as we can.
- activity.mType = translateIntegerRunType(runType->second);
- } else {
- activity.mType = runType->second;
- }
+ activity.mType = std::strtol(runType->second.c_str(), nullptr, 10);
  }
  if (auto runNumber = metadata.find(metadata_keys::runNumber); runNumber != metadata.end()) {
  activity.mId = std::strtol(runNumber->second.c_str(), nullptr, 10);
@@ -80,14 +73,8 @@ core::Activity asActivity(const std::map<std::string, std::string>& metadata, co
 core::Activity asActivity(const boost::property_tree::ptree& tree, const std::string& provenance)
 {
  core::Activity activity;
- if (auto runType = tree.get_optional<std::string>(metadata_keys::runType); runType.has_value()) {
- if (isUnsignedInteger(runType.value())) {
- // we probably got the former representation of run types, i.e. an integer. We convert it as best
- // as we can using O2's ECSDataAdapter
- activity.mType = parameters::GRPECS::RunTypeNames[std::stoi(runType.value())];
- } else {
- activity.mType = runType.value();
- }
+ if (auto runType = tree.get_optional<int>(metadata_keys::runType); runType.has_value()) {
+ activity.mType = runType.value();
  }
  if (auto runNumber = tree.get_optional<int>(metadata_keys::runNumber); runNumber.has_value()) {
  activity.mId = runNumber.value();

diff --git a/Framework/src/CustomParameters.cxx b/Framework/src/CustomParameters.cxx
@@ -71,7 +71,13 @@ std::string CustomParameters::at(const std::string& key, const std::string& runT
 
 std::string CustomParameters::at(const std::string& key, const Activity& activity) const
 {
- return at(key, activity.mType, activity.mBeamType);
+ // 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<std::string> CustomParameters::atOptional(const std::string& key, const std::string& runType, const std::string& beamType) const
@@ -93,7 +99,13 @@ std::optional<std::string> CustomParameters::atOptional(const std::string& key,
 
 std::optional<std::string> CustomParameters::atOptional(const std::string& key, const Activity& activity) const
 {
- return atOptional(key, activity.mType, activity.mBeamType);
+ // 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 atOptional(key, runTypeString, activity.mBeamType);
 }
 
 std::string CustomParameters::atOrDefaultValue(const std::string& key, std::string defaultValue, const std::string& runType, const std::string& beamType)
@@ -108,7 +120,12 @@ std::string CustomParameters::atOrDefaultValue(const std::string& key, std::stri
 std::string CustomParameters::atOrDefaultValue(const std::string& key, std::string defaultValue, const Activity& activity) const
 {
  try {
- return mCustomParameters.at(activity.mType).at(activity.mBeamType).at(key);
+ // Get the proper parameter for the given activity
+ const int runType = activity.mType; // get the type for this run as an int
+ // 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 };
+ return mCustomParameters.at(runTypeString).at(activity.mBeamType).at(key);
  } catch (const std::out_of_range& exc) {
  return defaultValue;
  }

diff --git a/Framework/src/InfrastructureSpecReader.cxx b/Framework/src/InfrastructureSpecReader.cxx
@@ -59,7 +59,7 @@ CommonSpec InfrastructureSpecReader::readSpecEntry<CommonSpec>(const std::string
  spec.database.emplace(key, value.get_value<std::string>());
  }
  spec.activityNumber = commonTree.get<int>("Activity.number", spec.activityNumber);
- spec.activityType = commonTree.get<std::string>("Activity.type", spec.activityType);
+ spec.activityType = commonTree.get<int>("Activity.type", spec.activityType);
  spec.activityPassName = commonTree.get<std::string>("Activity.passName", spec.activityPassName);
  spec.activityPeriodName = commonTree.get<std::string>("Activity.periodName", spec.activityPeriodName);
  spec.activityProvenance = commonTree.get<std::string>("Activity.provenance", spec.activityProvenance);

diff --git a/Framework/src/MonitorObject.cxx b/Framework/src/MonitorObject.cxx
@@ -42,7 +42,7 @@ MonitorObject::MonitorObject(TObject* object, const std::string& taskName, const
  mTaskName(taskName),
  mTaskClass(taskClass),
  mDetectorName(detectorName),
- mActivity(runNumber, "NONE", periodName, passName, provenance, gInvalidValidityInterval),
+ mActivity(runNumber, 0, periodName, passName, provenance, gInvalidValidityInterval),
  mIsOwner(true)
 {
 }

diff --git a/Framework/src/PostProcessingConfig.cxx b/Framework/src/PostProcessingConfig.cxx
@@ -31,7 +31,7 @@ PostProcessingConfig::PostProcessingConfig(const std::string& id, const boost::p
  ccdbUrl(config.get<std::string>("qc.config.conditionDB.url", "")),
  consulUrl(config.get<std::string>("qc.config.consul.url", "")),
  activity(config.get<int>("qc.config.Activity.number", 0),
- config.get<std::string>("qc.config.Activity.type", "NONE"),
+ config.get<int>("qc.config.Activity.type", 0),
  config.get<std::string>("qc.config.Activity.periodName", ""),
  config.get<std::string>("qc.config.Activity.passName", ""),
  config.get<std::string>("qc.config.Activity.provenance", "qc"),

diff --git a/Framework/src/QualityObject.cxx b/Framework/src/QualityObject.cxx
@@ -35,7 +35,7 @@ QualityObject::QualityObject(
  mPolicyName{ std::move(policyName) },
  mInputs{ std::move(inputs) },
  mMonitorObjectsNames{ std::move(monitorObjectsNames) },
- mActivity(runNumber, "NONE", "", "", "qc", gInvalidValidityInterval)
+ mActivity(runNumber, 0, "", "", "qc", gInvalidValidityInterval)
 {
  mQuality.addMetadata(std::move(metadata));
 }