Merge pull request #166 from mach3-software/feature_random

Random Tidy

.github/workflows/CDImage.yml

@@ -65,3 +65,11 @@ jobs:
         else
           docker push ghcr.io/${{ github.repository_owner }}/mach3:${{ matrix.tag_latest }}
         fi
+
+    - name: Delete old images
+      uses: actions/delete-package-versions@v5
+      with:
+        package-name: 'mach3'
+        package-type: 'container'
+        min-versions-to-keep: 5
+        delete-only-untagged-versions: 'true'

Doc/MaCh3DockerFiles/Alma9/Dockerfile

@@ -7,6 +7,7 @@ LABEL maintainer="The MaCh3 Collaboration"
 LABEL website="https://mach3-software.github.io/MaCh3/"
 LABEL compiler="GNU 11.3.1"
 LABEL root_version="v6.26.10"
+LABEL org.opencontainers.image.description="Official MaCh3 container"
 
 # Declare the build argument
 ARG MACH3_VERSION

Doc/MaCh3DockerFiles/Fedora32/Dockerfile

@@ -7,6 +7,7 @@ LABEL maintainer="The MaCh3 Collaboration"
 LABEL website="https://mach3-software.github.io/MaCh3/"
 LABEL compiler="GNU 10.1.1"
 LABEL root_version="v6.20.06"
+LABEL org.opencontainers.image.description="Official MaCh3 container"
 
 # Declare the build argument
 ARG MACH3_VERSION

Doc/MaCh3DockerFiles/Ubuntu22.04/Dockerfile

@@ -7,6 +7,7 @@ LABEL maintainer="The MaCh3 Collaboration"
 LABEL website="https://mach3-software.github.io/MaCh3/"
 LABEL compiler="GNU 11.4.0"
 LABEL root_version="v6.32.02"
+LABEL org.opencontainers.image.description="Official MaCh3 container"
 
 RUN apt update && apt upgrade -y
 

covariance/covarianceBase.cpp

@@ -475,7 +475,10 @@ void covarianceBase::throwParameters() {
   // First draw new randParams
   randomize();
 
-  if (!pca) {
+  // KS: We use PCA very rarely on top PCA functionality isn't implemented for this function.
+  // Use __builtin_expect to give compiler a hint which option is more likely, which should help
+  // with better optimisation. This isn't critical but more to have example
+  if (__builtin_expect(!pca, 1)) {
     MatrixVectorMulti(corr_throw, throwMatrixCholDecomp, randParams, _fNumPar);
 
     #ifdef MULTITHREAD
@@ -515,7 +518,7 @@ void covarianceBase::throwParameters() {
   }
   else
   {
-    MACH3LOG_CRITICAL("Hold on, you are trying to run Prior Predicitve Code with PCA, which is wrong");
+    MACH3LOG_CRITICAL("Hold on, you are trying to run Prior Predictive Code with PCA, which is wrong");
     MACH3LOG_CRITICAL("Sorry I have to kill you, I mean your job");
     throw MaCh3Exception(__FILE__ , __LINE__ );
   }

covariance/covarianceXsec.cpp

@@ -7,7 +7,6 @@
 covarianceXsec::covarianceXsec(const std::vector<std::string>& YAMLFile, const char *name, double threshold, int FirstPCA, int LastPCA)
                : covarianceBase(YAMLFile, name, threshold, FirstPCA, LastPCA){
 // ********************************************
-
   InitXsecFromConfig();
 
   //ETA - again this really doesn't need to be hear...
@@ -26,7 +25,7 @@ covarianceXsec::covarianceXsec(const std::vector<std::string>& YAMLFile, const c
 // ********************************************
 void covarianceXsec::InitXsecFromConfig() {
 // ********************************************
-  _fSystToGlobalSystIndexMap.resize(kSystTypes);
+  _fSystToGlobalSystIndexMap.resize(SystType::kSystTypes);
 
   _fDetID = std::vector<int>(_fNumPar);
   _fParamType = std::vector<SystType>(_fNumPar);
@@ -37,7 +36,7 @@ void covarianceXsec::InitXsecFromConfig() {
   SplineParams.reserve(_fNumPar);
 
   int i = 0;
-  unsigned int ParamCounter[kSystTypes] = {0};
+  unsigned int ParamCounter[SystType::kSystTypes] = {0};
   //ETA - read in the systematics. Would be good to add in some checks to make sure
   //that there are the correct number of entries i.e. are the _fNumPars for Names,
   //PreFitValues etc etc.
@@ -62,23 +61,23 @@ void covarianceXsec::InitXsecFromConfig() {
 
       //Insert the mapping from the spline index i.e. the length of _fSplineNames etc
       //to the Systematic index i.e. the counter for things like _fDetID and _fDetID
-      _fSystToGlobalSystIndexMap[kSpline].insert(std::make_pair(ParamCounter[kSpline], i));
-      ParamCounter[kSpline]++;
+      _fSystToGlobalSystIndexMap[SystType::kSpline].insert(std::make_pair(ParamCounter[SystType::kSpline], i));
+      ParamCounter[SystType::kSpline]++;
     } else if(param["Systematic"]["Type"].as<std::string>() == SystType_ToString(SystType::kNorm)) {
       _fParamType[i] = SystType::kNorm;
       NormParams.push_back(GetXsecNorm(param["Systematic"], i));
-      _fSystToGlobalSystIndexMap[kNorm].insert(std::make_pair(ParamCounter[kNorm], i));
-      ParamCounter[kNorm]++;
+      _fSystToGlobalSystIndexMap[SystType::kNorm].insert(std::make_pair(ParamCounter[SystType::kNorm], i));
+      ParamCounter[SystType::kNorm]++;
     }
     else if(param["Systematic"]["Type"].as<std::string>() == SystType_ToString(SystType::kFunc)){
       _fParamType[i] = SystType::kFunc;
-      _fSystToGlobalSystIndexMap[kFunc].insert(std::make_pair(ParamCounter[kFunc], i));
-      ParamCounter[kFunc]++;
+      _fSystToGlobalSystIndexMap[SystType::kFunc].insert(std::make_pair(ParamCounter[SystType::kFunc], i));
+      ParamCounter[SystType::kFunc]++;
     }
     else{
       MACH3LOG_ERROR("Given unrecognised systematic type: {}", param["Systematic"]["Type"].as<std::string>());
       std::string expectedTypes = "Expecting ";
-      for (int s = 0; s < kSystTypes; ++s) {
+      for (int s = 0; s < SystType::kSystTypes; ++s) {
         if (s > 0) expectedTypes += ", ";
         expectedTypes += SystType_ToString(static_cast<SystType>(s)) + "\"";
       }
@@ -90,8 +89,8 @@ void covarianceXsec::InitXsecFromConfig() {
   }
 
   //Add a sanity check,
-  if(_fSplineNames.size() != ParamCounter[kSpline]){
-    MACH3LOG_ERROR("_fSplineNames is of size {} but found {} spline parameters", _fSplineNames.size(), ParamCounter[kSpline]);
+  if(_fSplineNames.size() != ParamCounter[SystType::kSpline]){
+    MACH3LOG_ERROR("_fSplineNames is of size {} but found {} spline parameters", _fSplineNames.size(), ParamCounter[SystType::kSpline]);
     throw MaCh3Exception(__FILE__, __LINE__);
   }
   //KS We resized them above to all params to fight memory fragmentation, now let's resize to fit only allocated memory to save RAM
@@ -104,15 +103,15 @@ void covarianceXsec::InitXsecFromConfig() {
 // ********************************************
 covarianceXsec::~covarianceXsec() {
 // ********************************************
-
+  MACH3LOG_DEBUG("Deleting covarianceXsec");
 }
 
 // ********************************************
 // DB Grab the Spline Names for the relevant DetID
 const std::vector<std::string> covarianceXsec::GetSplineParsNamesFromDetID(const int DetID) {
 // ********************************************
   std::vector<std::string> returnVec;
-  for (auto &pair : _fSystToGlobalSystIndexMap[kSpline]) {
+  for (auto &pair : _fSystToGlobalSystIndexMap[SystType::kSpline]) {
     auto &SplineIndex = pair.first;
     auto &SystIndex = pair.second;
     if ((GetParDetID(SystIndex) & DetID )){
@@ -129,7 +128,7 @@ const std::vector< std::vector<int> > covarianceXsec::GetSplineModeVecFromDetID(
   std::vector< std::vector<int> > returnVec;
   //Need a counter or something to correctly get the index in _fSplineModes since it's not of length nPars
   //Should probably just make a std::map<std::string, int> for param name to FD spline index
-  for (auto &pair : _fSystToGlobalSystIndexMap[kSpline]) {
+  for (auto &pair : _fSystToGlobalSystIndexMap[SystType::kSpline]) {
     auto &SplineIndex = pair.first;
     auto &SystIndex = pair.second;
     if ((GetParDetID(SystIndex) & DetID)) { //If parameter applies to required DetID
@@ -384,7 +383,7 @@ void covarianceXsec::PrintNormParams() {
 // ********************************************
   // Output the normalisation parameters as a sanity check!
   MACH3LOG_INFO("Normalisation parameters:  {}", NormParams.size());
-  if(_fSystToGlobalSystIndexMap[kNorm].size() == 0) return;
+  if(_fSystToGlobalSystIndexMap[SystType::kNorm].size() == 0) return;
 
   //KS: Consider making some class producing table..
   MACH3LOG_INFO("┌────┬──────────┬────────────────────────────────────────┬────────────────────┬────────────────────┬────────────────────┐");
@@ -422,12 +421,12 @@ void covarianceXsec::PrintNormParams() {
 // ********************************************
 void covarianceXsec::PrintSplineParams() {
 // ********************************************
-  MACH3LOG_INFO("Spline parameters: {}", _fSystToGlobalSystIndexMap[kSpline].size());
-  if(_fSystToGlobalSystIndexMap[kSpline].size() == 0) return;
+  MACH3LOG_INFO("Spline parameters: {}", _fSystToGlobalSystIndexMap[SystType::kSpline].size());
+  if(_fSystToGlobalSystIndexMap[SystType::kSpline].size() == 0) return;
   MACH3LOG_INFO("=====================================================================================================================================================================");
   MACH3LOG_INFO("{:<4} {:<2} {:<40} {:<2} {:<40} {:<2} {:<20} {:<2} {:<20} {:<2} {:<20} {:<2}", "#", "|", "Name", "|", "Spline Name", "|", "Spline Interpolation", "|", "Low Knot Bound", "|", "Up Knot Bound", "|");
   MACH3LOG_INFO("---------------------------------------------------------------------------------------------------------------------------------------------------------------------");
-  for (auto &pair : _fSystToGlobalSystIndexMap[kSpline]) {
+  for (auto &pair : _fSystToGlobalSystIndexMap[SystType::kSpline]) {
     auto &SplineIndex = pair.first;
     auto &GlobalIndex = pair.second;
 
@@ -444,12 +443,12 @@ void covarianceXsec::PrintSplineParams() {
 // ********************************************
 void covarianceXsec::PrintFunctionalParams() {
 // ********************************************
-  MACH3LOG_INFO("Functional parameters: {}", _fSystToGlobalSystIndexMap[kFunc].size());
-  if(_fSystToGlobalSystIndexMap[kFunc].size() == 0) return;
+  MACH3LOG_INFO("Functional parameters: {}", _fSystToGlobalSystIndexMap[SystType::kFunc].size());
+  if(_fSystToGlobalSystIndexMap[SystType::kFunc].size() == 0) return;
   MACH3LOG_INFO("┌────┬──────────┬────────────────────────────────────────┐");
   MACH3LOG_INFO("│{0:4}│{1:10}│{2:40}│", "#", "Global #", "Name");
   MACH3LOG_INFO("├────┼──────────┼────────────────────────────────────────┤");
-  for (auto &pair : _fSystToGlobalSystIndexMap[kFunc]) {
+  for (auto &pair : _fSystToGlobalSystIndexMap[SystType::kFunc]) {
     auto &FuncIndex = pair.first;
     auto &GlobalIndex = pair.second;
     MACH3LOG_INFO("│{0:4}│{1:<10}│{2:40}│", std::to_string(FuncIndex), GlobalIndex, GetParFancyName(GlobalIndex));
@@ -570,7 +569,7 @@ void covarianceXsec::DumpMatrixToFile(const std::string& Name) {
     (*xsec_param_knot_weight_ub)[i] = +9999;
   }
 
-  for (auto &pair : _fSystToGlobalSystIndexMap[kSpline]) {
+  for (auto &pair : _fSystToGlobalSystIndexMap[SystType::kSpline]) {
     auto &SplineIndex = pair.first;
     auto &SystIndex = pair.second;
 

covariance/covarianceXsec.h

@@ -60,25 +60,25 @@ class covarianceXsec : public covarianceBase {
     /// @brief DB Grab the Spline Modes for the relevant DetID
     const std::vector< std::vector<int> > GetSplineModeVecFromDetID(const int DetID);
     /// @brief DB Grab the Spline Indices for the relevant DetID
-    const std::vector<int> GetSplineParsIndexFromDetID(const int DetID){return GetParsIndexFromDetID(DetID, kSpline);}
+    const std::vector<int> GetSplineParsIndexFromDetID(const int DetID){return GetParsIndexFromDetID(DetID, SystType::kSpline);}
     /// @brief ETA Grab the index of the spline relative to the _fSplineNames vector.
-    const std::vector<int> GetSplineSystIndexFromDetID(const int DetID){return GetSystIndexFromDetID(DetID, kSpline);};
+    const std::vector<int> GetSplineSystIndexFromDetID(const int DetID){return GetSystIndexFromDetID(DetID, SystType::kSpline);};
     /// @brief Grab the index of the syst relative to global numbering.
     /// @param Type Type of syst, for example kNorm, kSpline etc
     const std::vector<int> GetSystIndexFromDetID(const int DetID, const SystType Type);
 
     /// @brief DB Grab the Number of splines for the relevant DetID
-    int GetNumSplineParamsFromDetID(const int DetID){return GetNumParamsFromDetID(DetID, kSpline);}
+    int GetNumSplineParamsFromDetID(const int DetID){return GetNumParamsFromDetID(DetID, SystType::kSpline);}
 
     /// @brief DB Get norm/func parameters depending on given DetID
     const std::vector<XsecNorms4> GetNormParsFromDetID(const int DetID);
 
     /// @brief DB Grab the number of Normalisation parameters for the relevant DetID
-    int GetNumFuncParamsFromDetID(const int DetID){return GetNumParamsFromDetID(DetID, kFunc);}
+    int GetNumFuncParamsFromDetID(const int DetID){return GetNumParamsFromDetID(DetID, SystType::kFunc);}
     /// @brief DB Grab the Functional parameter names for the relevant DetID
-    const std::vector<std::string> GetFuncParsNamesFromDetID(const int DetID){return GetParsNamesFromDetID(DetID, kFunc);}
+    const std::vector<std::string> GetFuncParsNamesFromDetID(const int DetID){return GetParsNamesFromDetID(DetID, SystType::kFunc);}
     /// @brief DB Grab the Functional parameter indices for the relevant DetID
-    const std::vector<int> GetFuncParsIndexFromDetID(const int DetID){return GetParsIndexFromDetID(DetID, kFunc);}
+    const std::vector<int> GetFuncParsIndexFromDetID(const int DetID){return GetParsIndexFromDetID(DetID, SystType::kFunc);}
 
     /// @brief KS: For most covariances nominal and fparInit (prior) are the same, however for Xsec those can be different
     /// For example Sigma Var are done around nominal in ND280, no idea why though...

manager/Monitor.cpp

@@ -150,12 +150,11 @@ void GetDiskUsage() {
 
 // ************************
 // KS: Convoluted code to grab output from terminal to string
-std::string TerminalToString(const char* cmd) {
+std::string TerminalToString(std::string cmd) {
 // ************************
-
   std::array<char, 128> buffer;
   std::string result;
-  std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd, "r"), pclose);
+  std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd.c_str(), "r"), pclose);
   if (!pipe) {
     throw MaCh3Exception(__FILE__, __LINE__, "popen() failed!");
   }
@@ -211,7 +210,7 @@ void PrintProgressBar(const int Done, const int All){
 
 // ***************************************************************************
 //CW: Get memory, which is probably silly
-int getValue(std::string Type){ //Note: this value is in KB!
+int getValue(const std::string& Type){ //Note: this value is in KB!
 // ***************************************************************************
   std::ifstream file("/proc/self/status");
   int result = -1;
@@ -295,4 +294,5 @@ void MaCh3Usage(int argc, char **argv){
     throw MaCh3Exception(__FILE__, __LINE__);
   }
 }
-}
+
+} //end namespace

manager/Monitor.h

@@ -32,7 +32,7 @@ namespace MaCh3Utils {
   /// @brief KS: Convoluted code to grab output from terminal to string
   /// @param cmd The terminal command to execute.
   /// @return The output of the terminal command as a string.
-  std::string TerminalToString(const char* cmd);
+  std::string TerminalToString(std::string cmd);
   /// @brief KS: Check what CPU you are using
   void EstimateDataTransferRate(TChain* chain, const int entry);
   /// @brief KS: Find out about Disk usage
@@ -50,7 +50,7 @@ namespace MaCh3Utils {
   /// @param Type The type of system information to retrieve (e.g., RAM, CPU usage).
   /// @return The requested system information as an integer.
   /// @details This function fetches system information like RAM usage or other hardware details based on the specified type.
-  int getValue(std::string Type);
+  int getValue(const std::string& Type);
   /// @brief CW: Get memory, which is probably silly
   /// @param line The line of text to parse.
   /// @return The extracted memory value as an integer.

manager/manager.cpp

@@ -18,7 +18,7 @@ manager::manager(std::string const &filename)
 
   if (config["LikelihoodOptions"])
   {
-    std::string likelihood = GetFromManager<std::string>(config["LikelihoodOptions"]["TestStatistic"], "Barlow-Beeston");
+    auto likelihood = GetFromManager<std::string>(config["LikelihoodOptions"]["TestStatistic"], "Barlow-Beeston");
     if (likelihood == "Barlow-Beeston")                 mc_stat_llh = TestStatistic(kBarlowBeeston);
     else if (likelihood == "IceCube")                   mc_stat_llh = TestStatistic(kIceCube);
     else if (likelihood == "Poisson")                   mc_stat_llh = TestStatistic(kPoisson);

mcmc/FitterBase.cpp

@@ -28,7 +28,6 @@ FitterBase::FitterBase(manager * const man) : fitMan(man) {
   #endif
 
   std::string outfile = fitMan->raw()["General"]["OutputFile"].as<std::string>();
-
   // Save output every auto_save steps
   //you don't want this too often https://root.cern/root/html606/TTree_8cxx_source.html#l01229
   auto_save = fitMan->raw()["General"]["MCMC"]["AutoSave"].as<int>();
@@ -867,9 +866,9 @@ void FitterBase::Run2DLLHScan() {
   }
 
   // Number of points we do for each LLH scan
-  const int n_points = 20;
+  constexpr int n_points = 20;
   // We print 5 reweights
-  const int countwidth = double(n_points)/double(5);
+  constexpr int countwidth = double(n_points)/double(5);
 
   bool isxsec = false;
   // Loop over the covariance classes
@@ -1050,15 +1049,15 @@ void FitterBase::RunSigmaVar() {
   MACH3LOG_INFO("Starting Sigma Variation");
 
   // Number of variations we want
-  const int numVar = 5;
+  constexpr int numVar = 5;
   //-3 -1 0 +1 +3 sigma variation
-  const int sigmaArray[numVar] = {-3, -1, 0, 1, 3};
+  constexpr int sigmaArray[numVar] = {-3, -1, 0, 1, 3};
 
   outputFile->cd();
 
   //KS: this is only relevant if PlotByMode is turned on
   //Checking each mode is time consuming so we only consider one which are relevant for particular analysis
-  const int nRelevantModes = 2;
+  constexpr int nRelevantModes = 2;
   MaCh3Modes_t RelevantModes[nRelevantModes] = {Modes->GetMode("CCQE"), Modes->GetMode("2p2h")};
   bool DoByMode = GetFromManager<int>(fitMan->raw()["General"]["DoByMode"], false);