refactor: Tier 1 & 2 code quality improvements (v3.3.0 baseline)

.pre-commit-config.yaml

@@ -56,8 +56,8 @@ repos:
     hooks:
       - id: check-pragma-once
         name: Verify #pragma once in headers
-        entry: scripts/check-pragma-once.sh
-        language: script
+        entry: python scripts/check-pragma-once.py
+        language: python
         files: \.(h|hpp|hxx)$
         pass_filenames: true
 

CMakeLists.txt

@@ -343,6 +343,17 @@ include_directories(include)
 # Platform-specific compiler flags
 # NOTE: MSVC flags are applied per-target (see after add_library below) so that
 # FetchContent dependencies like GTest are not compiled with /W4.
+#
+# /utf-8 is the exception: it specifies source and execution character sets as
+# UTF-8 globally. This suppresses C4566 warnings that occur when source files
+# contain non-ASCII Unicode characters (Greek letters, math symbols, etc.) on
+# Windows systems with a non-UTF-8 ANSI code page (e.g. CP1252). It is safe
+# to apply to all targets including GTest because it only affects how MSVC
+# interprets source encoding — it has no effect on ASCII-only code.
+if(MSVC)
+    add_compile_options(/utf-8)
+endif()
+
 if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wpedantic -Wpointer-arith")
     # Note: -Wno-deprecated-declarations and -Wno-defaulted-function-deleted removed
@@ -508,6 +519,7 @@ set(LIBHMM_SOURCES
     src/common/common.cpp
     src/common/string_tokenizer.cpp
     src/common/numerical_stability.cpp
+    src/common/weighted_stats.cpp
     src/performance/transcendental_kernels.cpp
     src/distributions/distribution_base.cpp
     src/distributions/discrete_distribution.cpp

include/libhmm/calculators/calculator.h

@@ -72,6 +72,24 @@ class Calculator {
      * @param observations The new observation set
      */
     void setObservations(const ObservationSet &observations) { observations_ = observations; }
+
+    /// Fills logTrans (row-major) and logTransT (column-major/transposed) from the
+    /// HMM transition matrix. Resizes both matrices to numStates×numStates.
+    static void precompute_log_transitions(const Hmm &hmm, std::size_t numStates, Matrix &logTrans,
+                                           Matrix &logTransT) noexcept {
+        const Matrix &trans = hmm.getTrans();
+        logTrans.resize(numStates, numStates);
+        logTransT.resize(numStates, numStates);
+        for (std::size_t i = 0; i < numStates; ++i) {
+            for (std::size_t j = 0; j < numStates; ++j) {
+                const double a = trans(i, j);
+                const double logA =
+                    (a > 0.0) ? std::log(a) : -std::numeric_limits<double>::infinity();
+                logTrans(i, j) = logA;
+                logTransT(j, i) = logA;
+            }
+        }
+    }
 }; //class Calculator
 
 } // namespace libhmm

include/libhmm/distributions/beta_distribution.h

@@ -99,42 +99,10 @@ class BetaDistribution : public DistributionBase {
         updateCache();
     }
 
-    BetaDistribution(const BetaDistribution &other)
-        : DistributionBase{other}, alpha_{other.alpha_}, beta_{other.beta_},
-          logBeta_{other.logBeta_}, alphaMinus1_{other.alphaMinus1_},
-          betaMinus1_{other.betaMinus1_}, invBeta_{other.invBeta_} {}
-
-    BetaDistribution &operator=(const BetaDistribution &other) {
-        if (this != &other) {
-            DistributionBase::operator=(other);
-            alpha_ = other.alpha_;
-            beta_ = other.beta_;
-            logBeta_ = other.logBeta_;
-            alphaMinus1_ = other.alphaMinus1_;
-            betaMinus1_ = other.betaMinus1_;
-            invBeta_ = other.invBeta_;
-        }
-        return *this;
-    }
-
-    BetaDistribution(BetaDistribution &&other) noexcept
-        : DistributionBase{std::move(other)}, alpha_{other.alpha_}, beta_{other.beta_},
-          logBeta_{other.logBeta_}, alphaMinus1_{other.alphaMinus1_},
-          betaMinus1_{other.betaMinus1_}, invBeta_{other.invBeta_} {}
-
-    BetaDistribution &operator=(BetaDistribution &&other) noexcept {
-        if (this != &other) {
-            DistributionBase::operator=(std::move(other));
-            alpha_ = other.alpha_;
-            beta_ = other.beta_;
-            logBeta_ = other.logBeta_;
-            alphaMinus1_ = other.alphaMinus1_;
-            betaMinus1_ = other.betaMinus1_;
-            invBeta_ = other.invBeta_;
-        }
-        return *this;
-    }
-
+    BetaDistribution(const BetaDistribution &other) = default;
+    BetaDistribution &operator=(const BetaDistribution &other) = default;
+    BetaDistribution(BetaDistribution &&other) noexcept = default;
+    BetaDistribution &operator=(BetaDistribution &&other) noexcept = default;
     ~BetaDistribution() override = default;
 
     /**

include/libhmm/distributions/binomial_distribution.h

@@ -67,40 +67,10 @@ class BinomialDistribution : public DistributionBase {
         updateCache();
     }
 
-    BinomialDistribution(const BinomialDistribution &other)
-        : DistributionBase{other}, n_{other.n_}, p_{other.p_},
-          logFactorialCache_{other.logFactorialCache_}, logP_{other.logP_},
-          log1MinusP_{other.log1MinusP_} {}
-
-    BinomialDistribution &operator=(const BinomialDistribution &other) {
-        if (this != &other) {
-            DistributionBase::operator=(other);
-            n_ = other.n_;
-            p_ = other.p_;
-            logFactorialCache_ = other.logFactorialCache_;
-            logP_ = other.logP_;
-            log1MinusP_ = other.log1MinusP_;
-        }
-        return *this;
-    }
-
-    BinomialDistribution(BinomialDistribution &&other) noexcept
-        : DistributionBase{std::move(other)}, n_{other.n_}, p_{other.p_},
-          logFactorialCache_{std::move(other.logFactorialCache_)}, logP_{other.logP_},
-          log1MinusP_{other.log1MinusP_} {}
-
-    BinomialDistribution &operator=(BinomialDistribution &&other) noexcept {
-        if (this != &other) {
-            DistributionBase::operator=(std::move(other));
-            n_ = other.n_;
-            p_ = other.p_;
-            logFactorialCache_ = std::move(other.logFactorialCache_);
-            logP_ = other.logP_;
-            log1MinusP_ = other.log1MinusP_;
-        }
-        return *this;
-    }
-
+    BinomialDistribution(const BinomialDistribution &other) = default;
+    BinomialDistribution &operator=(const BinomialDistribution &other) = default;
+    BinomialDistribution(BinomialDistribution &&other) noexcept = default;
+    BinomialDistribution &operator=(BinomialDistribution &&other) noexcept = default;
     ~BinomialDistribution() override = default;
 
     [[nodiscard]] double getProbability(double value) const override;

include/libhmm/distributions/chi_squared_distribution.h

@@ -78,40 +78,10 @@ class ChiSquaredDistribution : public DistributionBase {
         updateCache();
     }
 
-    ChiSquaredDistribution(const ChiSquaredDistribution &other)
-        : DistributionBase{other}, degrees_of_freedom_{other.degrees_of_freedom_},
-          cached_log_gamma_half_k_{other.cached_log_gamma_half_k_},
-          cached_log_normalization_{other.cached_log_normalization_},
-          cached_half_k_minus_one_{other.cached_half_k_minus_one_} {}
-
-    ChiSquaredDistribution &operator=(const ChiSquaredDistribution &other) {
-        if (this != &other) {
-            DistributionBase::operator=(other);
-            degrees_of_freedom_ = other.degrees_of_freedom_;
-            cached_log_gamma_half_k_ = other.cached_log_gamma_half_k_;
-            cached_log_normalization_ = other.cached_log_normalization_;
-            cached_half_k_minus_one_ = other.cached_half_k_minus_one_;
-        }
-        return *this;
-    }
-
-    ChiSquaredDistribution(ChiSquaredDistribution &&other) noexcept
-        : DistributionBase{std::move(other)}, degrees_of_freedom_{other.degrees_of_freedom_},
-          cached_log_gamma_half_k_{other.cached_log_gamma_half_k_},
-          cached_log_normalization_{other.cached_log_normalization_},
-          cached_half_k_minus_one_{other.cached_half_k_minus_one_} {}
-
-    ChiSquaredDistribution &operator=(ChiSquaredDistribution &&other) noexcept {
-        if (this != &other) {
-            DistributionBase::operator=(std::move(other));
-            degrees_of_freedom_ = other.degrees_of_freedom_;
-            cached_log_gamma_half_k_ = other.cached_log_gamma_half_k_;
-            cached_log_normalization_ = other.cached_log_normalization_;
-            cached_half_k_minus_one_ = other.cached_half_k_minus_one_;
-        }
-        return *this;
-    }
-
+    ChiSquaredDistribution(const ChiSquaredDistribution &other) = default;
+    ChiSquaredDistribution &operator=(const ChiSquaredDistribution &other) = default;
+    ChiSquaredDistribution(ChiSquaredDistribution &&other) noexcept = default;
+    ChiSquaredDistribution &operator=(ChiSquaredDistribution &&other) noexcept = default;
     ~ChiSquaredDistribution() override = default;
 
     /**
@@ -193,14 +163,6 @@ class ChiSquaredDistribution : public DistributionBase {
      */
     double getMode() const noexcept { return std::max(0.0, degrees_of_freedom_ - 2.0); }
 
-    /**
-     * Create distribution from string representation
-     * @param str String representation
-     * @return ChiSquaredDistribution object
-     * @throws std::invalid_argument if string format is invalid
-     */
-    static ChiSquaredDistribution fromString(const std::string &str);
-
     /**
      * Equality comparison operator
      * @param other Other distribution to compare with

include/libhmm/distributions/discrete_distribution.h

@@ -127,52 +127,10 @@ class DiscreteDistribution : public DistributionBase {
         reset();
     }
 
-    DiscreteDistribution(const DiscreteDistribution &other)
-        : DistributionBase{other}, numSymbols_{other.numSymbols_}, pdf_{other.pdf_},
-          cachedSum_{other.cachedSum_}, cachedEntropy_{other.cachedEntropy_},
-          cachedLogProbs_{other.cachedLogProbs_}, cachedCDF_{other.cachedCDF_},
-          cachedMode_{other.cachedMode_}, cachedMaxProb_{other.cachedMaxProb_},
-          nonZeroIndices_{other.nonZeroIndices_} {}
-
-    DiscreteDistribution &operator=(const DiscreteDistribution &other) {
-        if (this != &other) {
-            DistributionBase::operator=(other);
-            numSymbols_ = other.numSymbols_;
-            pdf_ = other.pdf_;
-            cachedSum_ = other.cachedSum_;
-            cachedEntropy_ = other.cachedEntropy_;
-            cachedLogProbs_ = other.cachedLogProbs_;
-            cachedCDF_ = other.cachedCDF_;
-            cachedMode_ = other.cachedMode_;
-            cachedMaxProb_ = other.cachedMaxProb_;
-            nonZeroIndices_ = other.nonZeroIndices_;
-        }
-        return *this;
-    }
-
-    DiscreteDistribution(DiscreteDistribution &&other) noexcept
-        : DistributionBase{std::move(other)}, numSymbols_{other.numSymbols_},
-          pdf_{std::move(other.pdf_)}, cachedSum_{other.cachedSum_},
-          cachedEntropy_{other.cachedEntropy_}, cachedLogProbs_{std::move(other.cachedLogProbs_)},
-          cachedCDF_{std::move(other.cachedCDF_)}, cachedMode_{other.cachedMode_},
-          cachedMaxProb_{other.cachedMaxProb_}, nonZeroIndices_{std::move(other.nonZeroIndices_)} {}
-
-    DiscreteDistribution &operator=(DiscreteDistribution &&other) noexcept {
-        if (this != &other) {
-            DistributionBase::operator=(std::move(other));
-            numSymbols_ = other.numSymbols_;
-            pdf_ = std::move(other.pdf_);
-            cachedSum_ = other.cachedSum_;
-            cachedEntropy_ = other.cachedEntropy_;
-            cachedLogProbs_ = std::move(other.cachedLogProbs_);
-            cachedCDF_ = std::move(other.cachedCDF_);
-            cachedMode_ = other.cachedMode_;
-            cachedMaxProb_ = other.cachedMaxProb_;
-            nonZeroIndices_ = std::move(other.nonZeroIndices_);
-        }
-        return *this;
-    }
-
+    DiscreteDistribution(const DiscreteDistribution &other) = default;
+    DiscreteDistribution &operator=(const DiscreteDistribution &other) = default;
+    DiscreteDistribution(DiscreteDistribution &&other) noexcept = default;
+    DiscreteDistribution &operator=(DiscreteDistribution &&other) noexcept = default;
     ~DiscreteDistribution() override = default;
 
     /**

include/libhmm/distributions/distribution_io_utils.h

@@ -0,0 +1,30 @@
+#pragma once
+
+#include <algorithm>
+#include <cctype>
+#include <stdexcept>
+#include <string>
+#include <utility>
+
+namespace libhmm::detail {
+
+/// Splits a "name=value" token at the first '=', strips whitespace from both
+/// parts, and returns {name, value}.
+/// Throws std::invalid_argument (with context in the message) if no '=' found.
+[[nodiscard]] inline std::pair<std::string, std::string>
+parse_named_param(const std::string &param, const std::string &context) {
+    const auto eq = param.find('=');
+    if (eq == std::string::npos)
+        throw std::invalid_argument("Invalid " + context + " parameter format");
+    std::string name = param.substr(0, eq);
+    std::string value = param.substr(eq + 1);
+    const auto trim = [](std::string &s) {
+        s.erase(std::remove_if(s.begin(), s.end(), [](unsigned char c) { return std::isspace(c); }),
+                s.end());
+    };
+    trim(name);
+    trim(value);
+    return {std::move(name), std::move(value)};
+}
+
+} // namespace libhmm::detail

include/libhmm/distributions/exponential_distribution.h

@@ -88,43 +88,10 @@ class ExponentialDistribution : public DistributionBase {
         updateCache();
     }
 
-    /**
-     * Copy constructor
-     */
-    ExponentialDistribution(const ExponentialDistribution &other)
-        : DistributionBase{other}, lambda_{other.lambda_}, logLambda_{other.logLambda_},
-          invLambda_{other.invLambda_}, negLambda_{other.negLambda_},
-          invLambdaSquared_{other.invLambdaSquared_} {}
-
-    ExponentialDistribution &operator=(const ExponentialDistribution &other) {
-        if (this != &other) {
-            DistributionBase::operator=(other);
-            lambda_ = other.lambda_;
-            logLambda_ = other.logLambda_;
-            invLambda_ = other.invLambda_;
-            negLambda_ = other.negLambda_;
-            invLambdaSquared_ = other.invLambdaSquared_;
-        }
-        return *this;
-    }
-
-    ExponentialDistribution(ExponentialDistribution &&other) noexcept
-        : DistributionBase{std::move(other)}, lambda_{other.lambda_}, logLambda_{other.logLambda_},
-          invLambda_{other.invLambda_}, negLambda_{other.negLambda_},
-          invLambdaSquared_{other.invLambdaSquared_} {}
-
-    ExponentialDistribution &operator=(ExponentialDistribution &&other) noexcept {
-        if (this != &other) {
-            DistributionBase::operator=(std::move(other));
-            lambda_ = other.lambda_;
-            logLambda_ = other.logLambda_;
-            invLambda_ = other.invLambda_;
-            negLambda_ = other.negLambda_;
-            invLambdaSquared_ = other.invLambdaSquared_;
-        }
-        return *this;
-    }
-
+    ExponentialDistribution(const ExponentialDistribution &other) = default;
+    ExponentialDistribution &operator=(const ExponentialDistribution &other) = default;
+    ExponentialDistribution(ExponentialDistribution &&other) noexcept = default;
+    ExponentialDistribution &operator=(ExponentialDistribution &&other) noexcept = default;
     ~ExponentialDistribution() override = default;
 
     /**