From af6ac44f3c3e2043e136931c8931b93c260809d1 Mon Sep 17 00:00:00 2001 From: Kamil Skwarczynski Date: Sun, 13 Oct 2024 15:10:49 +0000 Subject: [PATCH 1/7] store fancy desription of contianers --- .github/workflows/CDImage.yml | 7 +++++++ Doc/MaCh3DockerFiles/Alma9/Dockerfile | 1 + Doc/MaCh3DockerFiles/Fedora32/Dockerfile | 1 + Doc/MaCh3DockerFiles/Ubuntu22.04/Dockerfile | 1 + 4 files changed, 10 insertions(+) diff --git a/.github/workflows/CDImage.yml b/.github/workflows/CDImage.yml index 0a82d7a9e..6da89f096 100644 --- a/.github/workflows/CDImage.yml +++ b/.github/workflows/CDImage.yml @@ -65,3 +65,10 @@ jobs: else docker push ghcr.io/${{ github.repository_owner }}/mach3:${{ matrix.tag_latest }} fi + + - uses: actions/delete-package-versions@v5 + with: + package-name: 'mach3' + package-type: 'container' + min-versions-to-keep: 5 + delete-only-untagged-versions: 'true' diff --git a/Doc/MaCh3DockerFiles/Alma9/Dockerfile b/Doc/MaCh3DockerFiles/Alma9/Dockerfile index 21bf9fb86..f299eac0d 100644 --- a/Doc/MaCh3DockerFiles/Alma9/Dockerfile +++ b/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 diff --git a/Doc/MaCh3DockerFiles/Fedora32/Dockerfile b/Doc/MaCh3DockerFiles/Fedora32/Dockerfile index a9e17109e..dd8cf1f7e 100644 --- a/Doc/MaCh3DockerFiles/Fedora32/Dockerfile +++ b/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 diff --git a/Doc/MaCh3DockerFiles/Ubuntu22.04/Dockerfile b/Doc/MaCh3DockerFiles/Ubuntu22.04/Dockerfile index d02d6f31d..ea571ec8c 100644 --- a/Doc/MaCh3DockerFiles/Ubuntu22.04/Dockerfile +++ b/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 From bcc9240ffd023eefa96c06fd0298b208cf23d615 Mon Sep 17 00:00:00 2001 From: Kamil Skwarczynski Date: Sun, 13 Oct 2024 15:11:16 +0000 Subject: [PATCH 2/7] remove dperacated funciton --- samplePDF/Structs.h | 47 +++++++++++----------- splines/SplineStructs.h | 89 ----------------------------------------- 2 files changed, 23 insertions(+), 113 deletions(-) diff --git a/samplePDF/Structs.h b/samplePDF/Structs.h index 2497947db..7f6df71da 100644 --- a/samplePDF/Structs.h +++ b/samplePDF/Structs.h @@ -135,12 +135,12 @@ inline std::string GetTF1(const SplineInterpolation i) { // ************************************************** std::string Func = ""; switch(i) { - case kLinearFunc: + case SplineInterpolation::kLinearFunc: Func = "([1]+[0]*x)"; break; default: std::cerr << "UNKNOWN SPECIFIED!" << std::endl; - std::cerr << "You gave " << i << std::endl; + std::cerr << "You gave " << static_cast(i) << std::endl; std::cerr << __FILE__ << ":" << __LINE__ << std::endl; throw; } @@ -155,25 +155,25 @@ inline RespFuncType SplineInterpolation_ToRespFuncType(const SplineInterpolation RespFuncType Type = kRespFuncTypes; switch(i) { // TSpline3 (third order spline in ROOT) - case kTSpline3: - Type = kTSpline3_red; + case SplineInterpolation::kTSpline3: + Type = RespFuncType::kTSpline3_red; break; - case kLinear: - Type = kTSpline3_red; + case SplineInterpolation::kLinear: + Type = RespFuncType::kTSpline3_red; break; - case kMonotonic: - Type = kTSpline3_red; + case SplineInterpolation::kMonotonic: + Type = RespFuncType::kTSpline3_red; break; // (Experimental) Akima_Spline (crd order spline which is allowed to be discontinuous in 2nd deriv) - case kAkima: - Type = kTSpline3_red; + case SplineInterpolation::kAkima: + Type = RespFuncType::kTSpline3_red; break; - case kLinearFunc: - Type = kTF1_red; + case SplineInterpolation::kLinearFunc: + Type = RespFuncType::kTF1_red; break; default: std::cerr << "UNKNOWN SPLINE INTERPOLATION SPECIFIED!" << std::endl; - std::cerr << "You gave " << i << std::endl; + std::cerr << "You gave " << static_cast(i) << std::endl; std::cerr << __FILE__ << ":" << __LINE__ << std::endl; throw; } @@ -187,25 +187,25 @@ inline std::string SplineInterpolation_ToString(const SplineInterpolation i) { std::string name = ""; switch(i) { // TSpline3 (third order spline in ROOT) - case kTSpline3: + case SplineInterpolation::kTSpline3: name = "TSpline3"; break; - case kLinear: + case SplineInterpolation::kLinear: name = "Linear"; break; - case kMonotonic: + case SplineInterpolation::kMonotonic: name = "Monotonic"; break; // (Experimental) Akima_Spline (crd order spline which is allowed to be discontinuous in 2nd deriv) - case kAkima: + case SplineInterpolation::kAkima: name = "Akima"; break; - case kLinearFunc: + case SplineInterpolation::kLinearFunc: name = "LinearFunc"; break; default: std::cerr << "UNKNOWN SPLINE INTERPOLATION SPECIFIED!" << std::endl; - std::cerr << "You gave " << i << std::endl; + std::cerr << "You gave " << static_cast(i) << std::endl; std::cerr << __FILE__ << ":" << __LINE__ << std::endl; throw; } @@ -220,7 +220,6 @@ enum SystType { kSystTypes //!< This only enumerates }; - // ******************* /// @brief KS: Struct holding info about Spline Systematics struct XsecSplines1 { @@ -243,18 +242,18 @@ inline std::string SystType_ToString(const SystType i) { // ************************************************** std::string name = ""; switch(i) { - case kNorm: + case SystType::kNorm: name = "Norm"; break; - case kSpline: + case SystType::kSpline: name = "Spline"; break; - case kFunc: + case SystType::kFunc: name = "Functional"; break; default: std::cerr << "UNKNOWN SYST TYPE SPECIFIED!" << std::endl; - std::cerr << "You gave " << i << std::endl; + std::cerr << "You gave " << static_cast(i) << std::endl; std::cerr << __FILE__ << ":" << __LINE__ << std::endl; throw; } diff --git a/splines/SplineStructs.h b/splines/SplineStructs.h index 856a4913a..42845c719 100644 --- a/splines/SplineStructs.h +++ b/splines/SplineStructs.h @@ -719,95 +719,6 @@ class TSpline3_red: public TResponseFunction_red { _float_ *YResp; }; -// ************************ -/// @brief CW: Truncated spline class -class Truncated_Spline: public TSpline3_red { -// ************************ -// cubic spline which is flat (returns y_first or y_last) if x outside of knot range -public: - /// @brief Empty constructor - Truncated_Spline() - :TSpline3_red() - { - } - - /// @brief The constructor that takes a TSpline3 pointer and copies in to memory - Truncated_Spline(TSpline3* &spline) - :TSpline3_red(spline) - { - } - - /// @brief Empty destructor - ~Truncated_Spline() - { - } - - /// @brief Find the segment relevant to this variation in x - /// See root/hist/hist/src/TSpline3::FindX(double) or SplineMonolith::FindSplineSegment - inline int FindX(double x) { - // The segment we're interested in (klow in ROOT code) - int segment = 0; - int kHigh = nPoints-1; - // If the variation is below the lowest saved spline point - if (x <= XPos[0]){ - segment = -1; - // If the variation is above the highest saved spline point - } else if (x >= XPos[nPoints-1]) { - segment = -2; - // If the variation is between the maximum and minimum, perform a binary search - } else { - // The top point we've got - int kHalf = 0; - // While there is still a difference in the points (we haven't yet found the segment) - // This is a binary search, incrementing segment and decrementing kHalf until we've found the segment - while (kHigh - segment > 1) { - // Increment the half-step - kHalf = (segment + kHigh)/2; - // If our variation is above the kHalf, set the segment to kHalf - if (x > XPos[kHalf]) { - segment = kHalf; - // Else move kHigh down - } else { - kHigh = kHalf; - } - } // End the while: we've now done our binary search - } // End the else: we've now found our point - if (segment >= nPoints-1 && nPoints > 1) segment = nPoints-2; - return segment; - } - - /// @brief Evaluate the weight from a variation - inline double Eval(double var) { - // Get the segment for this variation - int segment = FindX(var); - // The get the coefficients for this variation - _float_ x = -999.99, y = -999.99, b = -999.99, c = -999.99, d = -999.99; - - if(segment >= 0){ - GetCoeff(segment, x, y, b, c, d); - } - - // if var is outside of the defined range, set the coefficients to 0 so that Eval just returns the value at the end point of the spline - else if(segment == -1){ - GetKnot(0, x, y); - b = 0.0; - c = 0.0; - d = 0.0; - } - else if(segment == -2){ - GetKnot(nPoints-1, x, y); - b = 0.0; - c = 0.0; - d = 0.0; - } - - double dx = var - x; - // Evaluate the third order polynomial - double weight = y+dx*(b+dx*(c+d*dx)); - return weight; - } -}; - // ***************************************** /// @brief CW: Helper function used in the constructor, tests to see if the spline is flat /// @param spl pointer to TSpline3_red that will be checked From 62087e1062e1b581a619b3ff6ebc8377b0f6a89b Mon Sep 17 00:00:00 2001 From: Kamil Skwarczynski Date: Sun, 13 Oct 2024 15:11:37 +0000 Subject: [PATCH 3/7] try using enums more safely --- covariance/covarianceXsec.cpp | 45 +++++++++++++++++------------------ covariance/covarianceXsec.h | 12 +++++----- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/covariance/covarianceXsec.cpp b/covariance/covarianceXsec.cpp index 5c105ab75..547645fea 100644 --- a/covariance/covarianceXsec.cpp +++ b/covariance/covarianceXsec.cpp @@ -7,7 +7,6 @@ covarianceXsec::covarianceXsec(const std::vector& 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& YAMLFile, const c // ******************************************** void covarianceXsec::InitXsecFromConfig() { // ******************************************** - _fSystToGlobalSystIndexMap.resize(kSystTypes); + _fSystToGlobalSystIndexMap.resize(SystType::kSystTypes); _fDetID = std::vector(_fNumPar); _fParamType = std::vector(_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() == 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() == 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 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(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,7 +103,7 @@ void covarianceXsec::InitXsecFromConfig() { // ******************************************** covarianceXsec::~covarianceXsec() { // ******************************************** - + MACH3LOG_DEBUG("Deleting covarianceXsec"); } // ******************************************** @@ -112,7 +111,7 @@ covarianceXsec::~covarianceXsec() { const std::vector covarianceXsec::GetSplineParsNamesFromDetID(const int DetID) { // ******************************************** std::vector 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 > covarianceXsec::GetSplineModeVecFromDetID( std::vector< std::vector > 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 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; diff --git a/covariance/covarianceXsec.h b/covariance/covarianceXsec.h index 828f95929..159eb9ab3 100644 --- a/covariance/covarianceXsec.h +++ b/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 > GetSplineModeVecFromDetID(const int DetID); /// @brief DB Grab the Spline Indices for the relevant DetID - const std::vector GetSplineParsIndexFromDetID(const int DetID){return GetParsIndexFromDetID(DetID, kSpline);} + const std::vector 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 GetSplineSystIndexFromDetID(const int DetID){return GetSystIndexFromDetID(DetID, kSpline);}; + const std::vector 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 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 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 GetFuncParsNamesFromDetID(const int DetID){return GetParsNamesFromDetID(DetID, kFunc);} + const std::vector GetFuncParsNamesFromDetID(const int DetID){return GetParsNamesFromDetID(DetID, SystType::kFunc);} /// @brief DB Grab the Functional parameter indices for the relevant DetID - const std::vector GetFuncParsIndexFromDetID(const int DetID){return GetParsIndexFromDetID(DetID, kFunc);} + const std::vector 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... From 07b0fe410659fcad3b6492fee004c6b525033d35 Mon Sep 17 00:00:00 2001 From: Kamil Skwarczynski Date: Sun, 13 Oct 2024 15:12:00 +0000 Subject: [PATCH 4/7] use constexpr when we can --- manager/Monitor.cpp | 3 +- manager/manager.cpp | 2 +- mcmc/FitterBase.cpp | 11 +++---- mcmc/MCMCProcessor.cpp | 75 ++++++++++++++++++------------------------ 4 files changed, 40 insertions(+), 51 deletions(-) diff --git a/manager/Monitor.cpp b/manager/Monitor.cpp index 559c8e9a3..695322c43 100644 --- a/manager/Monitor.cpp +++ b/manager/Monitor.cpp @@ -295,4 +295,5 @@ void MaCh3Usage(int argc, char **argv){ throw MaCh3Exception(__FILE__, __LINE__); } } -} + +} //end namespace diff --git a/manager/manager.cpp b/manager/manager.cpp index 4e43b2fff..c85a784d8 100644 --- a/manager/manager.cpp +++ b/manager/manager.cpp @@ -18,7 +18,7 @@ manager::manager(std::string const &filename) if (config["LikelihoodOptions"]) { - std::string likelihood = GetFromManager(config["LikelihoodOptions"]["TestStatistic"], "Barlow-Beeston"); + auto likelihood = GetFromManager(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); diff --git a/mcmc/FitterBase.cpp b/mcmc/FitterBase.cpp index 2bd28b118..113b0e26e 100644 --- a/mcmc/FitterBase.cpp +++ b/mcmc/FitterBase.cpp @@ -28,7 +28,6 @@ FitterBase::FitterBase(manager * const man) : fitMan(man) { #endif std::string outfile = fitMan->raw()["General"]["OutputFile"].as(); - // 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(); @@ -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(fitMan->raw()["General"]["DoByMode"], false); diff --git a/mcmc/MCMCProcessor.cpp b/mcmc/MCMCProcessor.cpp index bf6e92015..55e9a2e5b 100644 --- a/mcmc/MCMCProcessor.cpp +++ b/mcmc/MCMCProcessor.cpp @@ -437,13 +437,14 @@ void MCMCProcessor::MakePostfit() { PostDir->Close(); delete PostDir; + PostHistDir->Close(); + delete PostHistDir; } // Have now written the postfit projections // ******************* //CW: Draw the postfit void MCMCProcessor::DrawPostfit() { // ******************* - if (OutputFile == nullptr) MakeOutputFile(); // Make the prefit plot @@ -860,7 +861,6 @@ void MCMCProcessor::MakeCredibleIntervals(const std::vector& CredibleInt Posterior->SetLeftMargin(LeftMargin); } - // ********************* // Make fancy violin plots void MCMCProcessor::MakeViolin() { @@ -884,7 +884,7 @@ void MCMCProcessor::MakeViolin() { hviolin = new TH2D("hviolin", "hviolin", nDraw, 0, nDraw, vBins, mini_y, maxi_y); //KS: Prior has larger errors so we increase range and number of bins - const int PriorFactor = 4; + constexpr int PriorFactor = 4; hviolin_prior = new TH2D("hviolin_prior", "hviolin_prior", nDraw, 0, nDraw, PriorFactor*vBins, PriorFactor*mini_y, PriorFactor*maxi_y); TRandom3* rand = new TRandom3(0); @@ -949,7 +949,7 @@ void MCMCProcessor::MakeViolin() { MACH3LOG_INFO("Making Violin plot took {:.2f}s to finish for {} steps", clock.RealTime(), nEntries); //KS: Tells how many parameters in one canvas we want - const int IntervalsSize = 10; + constexpr int IntervalsSize = 10; const int NIntervals = nDraw/IntervalsSize; hviolin->GetYaxis()->SetTitle("Parameter Value"); @@ -1008,7 +1008,6 @@ void MCMCProcessor::MakeViolin() { // Make the post-fit covariance matrix in all dimensions void MCMCProcessor::MakeCovariance() { // ********************* - if (OutputFile == nullptr) MakeOutputFile(); bool HaveMadeDiagonal = false; @@ -1028,15 +1027,12 @@ void MCMCProcessor::MakeCovariance() { if (HaveMadeDiagonal == false) { MakePostfit(); } - gStyle->SetPalette(55); - - int covBinning = nDraw; // Now we are sure we have the diagonal elements, let's make the off-diagonals - for (int i = 0; i < covBinning; ++i) + for (int i = 0; i < nDraw; ++i) { - if (i % (covBinning/5) == 0) - MaCh3Utils::PrintProgressBar(i, covBinning); + if (i % (nDraw/5) == 0) + MaCh3Utils::PrintProgressBar(i, nDraw); TString Title_i = ""; double Prior_i, PriorError; @@ -1238,12 +1234,10 @@ void MCMCProcessor::MakeCovariance_MP(bool Mute) { if(!CacheMCMC) CacheSteps(); - int covBinning = nDraw; - bool HaveMadeDiagonal = false; // Check that the diagonal entries have been filled // i.e. MakePostfit() has been called - for (int i = 0; i < covBinning; ++i) { + for (int i = 0; i < nDraw; ++i) { if ((*Covariance)(i,i) == _UNDEF_) { HaveMadeDiagonal = false; MACH3LOG_WARN("Have not run diagonal elements in covariance, will do so now by calling MakePostfit()"); @@ -1263,7 +1257,7 @@ void MCMCProcessor::MakeCovariance_MP(bool Mute) { #ifdef MULTITHREAD #pragma omp parallel for #endif - for (int i = 0; i < covBinning; ++i) + for (int i = 0; i < nDraw; ++i) { for (int j = 0; j <= i; ++j) { @@ -1309,7 +1303,7 @@ void MCMCProcessor::MakeCovariance_MP(bool Mute) { if(printToPDF) { Posterior->cd(); - for (int i = 0; i < covBinning; ++i) + for (int i = 0; i < nDraw; ++i) { for (int j = 0; j <= i; ++j) { @@ -1416,19 +1410,17 @@ void MCMCProcessor::MakeSubOptimality(const int NIntervals) { // Make the covariance plots void MCMCProcessor::DrawCovariance() { // ********************* - const double RightMargin = Posterior->GetRightMargin(); Posterior->SetRightMargin(0.15); - int covBinning = nDraw; // The Covariance matrix from the fit - TH2D* hCov = new TH2D("hCov", "hCov", covBinning, 0, covBinning, covBinning, 0, covBinning); + TH2D* hCov = new TH2D("hCov", "hCov", nDraw, 0, nDraw, nDraw, 0, nDraw); hCov->GetZaxis()->SetTitle("Covariance"); // The Covariance matrix square root, with correct sign - TH2D* hCovSq = new TH2D("hCovSq", "hCovSq", covBinning, 0, covBinning, covBinning, 0, covBinning); + TH2D* hCovSq = new TH2D("hCovSq", "hCovSq", nDraw, 0, nDraw, nDraw, 0, nDraw); hCovSq->GetZaxis()->SetTitle("Covariance"); // The Correlation - TH2D* hCorr = new TH2D("hCorr", "hCorr", covBinning, 0, covBinning, covBinning, 0, covBinning); + TH2D* hCorr = new TH2D("hCorr", "hCorr", nDraw, 0, nDraw, nDraw, 0, nDraw); hCorr->GetZaxis()->SetTitle("Correlation"); hCorr->SetMinimum(-1); hCorr->SetMaximum(1); @@ -1440,7 +1432,7 @@ void MCMCProcessor::DrawCovariance() { hCorr->GetYaxis()->SetLabelSize(0.015); // Loop over the Covariance matrix entries - for (int i = 0; i < covBinning; ++i) + for (int i = 0; i < nDraw; ++i) { TString titlex = ""; double nom, err; @@ -1450,17 +1442,16 @@ void MCMCProcessor::DrawCovariance() { hCovSq->GetXaxis()->SetBinLabel(i+1, titlex); hCorr->GetXaxis()->SetBinLabel(i+1, titlex); - for (int j = 0; j < covBinning; ++j) + for (int j = 0; j < nDraw; ++j) { // The value of the Covariance - double cov = (*Covariance)(i,j); - double corr = (*Correlation)(i,j); + const double cov = (*Covariance)(i,j); + const double corr = (*Correlation)(i,j); hCov->SetBinContent(i+1, j+1, cov); hCovSq->SetBinContent(i+1, j+1, ((cov > 0) - (cov < 0))*std::sqrt(std::fabs(cov))); hCorr->SetBinContent(i+1, j+1, corr); - TString titley = ""; double nom_j, err_j; GetNthParameter(j, nom_j, err_j, titley); @@ -1526,10 +1517,10 @@ void MCMCProcessor::DrawCorrelations1D() { Posterior->SetBottomMargin(0.2); gStyle->SetOptTitle(1); - const int Nhists = 3; + constexpr int Nhists = 3; //KS: Highest value is just meant bo be sliglhy higher than 1 to catch >, - const double Thresholds[Nhists+1] = {0, 0.25, 0.5, 1.0001}; - const Color_t CorrColours[Nhists] = {kRed-10, kRed-6, kRed}; + constexpr double Thresholds[Nhists+1] = {0, 0.25, 0.5, 1.0001}; + constexpr Color_t CorrColours[Nhists] = {kRed-10, kRed-6, kRed}; //KS: This strore neccesary entires for stripped covariance which strore only "menaingfull correlations std::vector> CorrOfInterest; @@ -1622,7 +1613,7 @@ void MCMCProcessor::DrawCorrelations1D() { delete leg; } - //KS: Plot only meaninfull correlations + //KS: Plot only meaningful correlations for(int i = 0; i < nDraw; i++) { const int size = CorrOfInterest[i].size(); @@ -2752,7 +2743,6 @@ void MCMCProcessor::SetStepCut(const int Cuts) { BurnInCut = Cuts; } - // *************** // Pass central value void MCMCProcessor::GetNthParameter(const int param, double &Prior, double &PriorError, TString &Title){ @@ -3817,9 +3807,9 @@ void MCMCProcessor::CalculateESS(const int nLags, double** LagL) { TVectorD* SamplingEfficiency = new TVectorD(nDraw); double *TempDenominator = new double[nDraw](); - const int Nhists = 5; - const double Thresholds[Nhists+1] = {1, 0.02, 0.005, 0.001, 0.0001, 0.0}; - const Color_t ESSColours[Nhists] = {kGreen, kGreen+2, kYellow, kOrange, kRed}; + constexpr int Nhists = 5; + constexpr double Thresholds[Nhists + 1] = {1, 0.02, 0.005, 0.001, 0.0001, 0.0}; + constexpr Color_t ESSColours[Nhists] = {kGreen, kGreen + 2, kYellow, kOrange, kRed}; //KS: This histogram is inspired by the following: @cite gabry2024visual TH1D **EffectiveSampleSizeHist = new TH1D*[Nhists](); @@ -4117,7 +4107,6 @@ void MCMCProcessor::PowerSpectrumAnalysis() { const int end = N_Coeffs/2-1; const int v_size = end - start; - int nPrams = nDraw; /// @todo KS: Code is awfully slow... I know how to make it faster (GPU scream in a distant) but for now just make it for two params, bit hacky sry... nPrams = 2; @@ -4236,12 +4225,12 @@ void MCMCProcessor::GewekeDiagnostic() { int* DenomCounterUp = new int[nDraw](); const double Threshold = 0.5 * nSteps; - //KS: Select values betwen which you want to scan, for example 0 means 0% burn in and 1 100% burn in. - const double LowerThreshold = 0; - const double UpperThreshold = 1.0; + //KS: Select values between which you want to scan, for example 0 means 0% burn in and 1 100% burn in. + constexpr double LowerThreshold = 0; + constexpr double UpperThreshold = 1.0; // Tells how many intervals between thresholds we want to check - const int NChecks = 100; - const double Division = (UpperThreshold - LowerThreshold)/NChecks; + constexpr int NChecks = 100; + constexpr double Division = (UpperThreshold - LowerThreshold)/NChecks; TH1D** GewekePlots = new TH1D*[nDraw]; for (int j = 0; j < nDraw; ++j) @@ -4410,9 +4399,9 @@ void MCMCProcessor::AcceptanceProbabilities() { BatchedAcceptanceProblot->GetXaxis()->SetBinLabel(i+1, ss.str().c_str()); } -#ifdef MULTITHREAD -#pragma omp parallel for -#endif + #ifdef MULTITHREAD + #pragma omp parallel for + #endif for (int i = 0; i < nEntries; ++i) { // Set bin content for the i-th bin to the parameter values AcceptanceProbPlot->SetBinContent(i, AccProbValues[i]); From fee469197bc553f76a598019acf88e7de9ed8121 Mon Sep 17 00:00:00 2001 From: Kamil Skwarczynski Date: Sun, 13 Oct 2024 15:19:26 +0000 Subject: [PATCH 5/7] minor comment --- samplePDF/Structs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samplePDF/Structs.h b/samplePDF/Structs.h index 7f6df71da..a78fd94ee 100644 --- a/samplePDF/Structs.h +++ b/samplePDF/Structs.h @@ -127,7 +127,6 @@ enum SplineInterpolation { kSplineInterpolations //!< This only enumerates }; - // ************************************************** /// @brief Get function for TF1_red /// @param i Interpolation type @@ -213,6 +212,7 @@ inline std::string SplineInterpolation_ToString(const SplineInterpolation i) { } /// Make an enum of systematic type recognised by covariance class +/// @todo KS: Consider using enum class, it is generally recommended as safer. It will require many static_cast enum SystType { kNorm, //!< For normalisation parameters kSpline, //!< For splined parameters (1D) From afd213a217171534af48ca2424dd652bb9aa7b2a Mon Sep 17 00:00:00 2001 From: Kamil Skwarczynski Date: Mon, 14 Oct 2024 07:38:55 +0000 Subject: [PATCH 6/7] minor tweaks --- covariance/covarianceBase.cpp | 7 +++++-- manager/Monitor.cpp | 7 +++---- manager/Monitor.h | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/covariance/covarianceBase.cpp b/covariance/covarianceBase.cpp index 3cd8cf149..eb45a83a1 100644 --- a/covariance/covarianceBase.cpp +++ b/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__ ); } diff --git a/manager/Monitor.cpp b/manager/Monitor.cpp index 695322c43..90a7f0432 100644 --- a/manager/Monitor.cpp +++ b/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 buffer; std::string result; - std::unique_ptr pipe(popen(cmd, "r"), pclose); + std::unique_ptr 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; diff --git a/manager/Monitor.h b/manager/Monitor.h index 9f783e111..dae7a72bd 100644 --- a/manager/Monitor.h +++ b/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. From 947442f8673d9251d7079236c0a6a28b0d267419 Mon Sep 17 00:00:00 2001 From: Kamil Skwarczynski Date: Mon, 14 Oct 2024 07:59:29 +0000 Subject: [PATCH 7/7] add name for step --- .github/workflows/CDImage.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/CDImage.yml b/.github/workflows/CDImage.yml index 6da89f096..0bbe2034c 100644 --- a/.github/workflows/CDImage.yml +++ b/.github/workflows/CDImage.yml @@ -66,7 +66,8 @@ jobs: docker push ghcr.io/${{ github.repository_owner }}/mach3:${{ matrix.tag_latest }} fi - - uses: actions/delete-package-versions@v5 + - name: Delete old images + uses: actions/delete-package-versions@v5 with: package-name: 'mach3' package-type: 'container'