From 47a6e5ec1f49b98e1adf5c0723ceb847c552a68e Mon Sep 17 00:00:00 2001 From: patwie Date: Sun, 28 Jun 2026 21:55:00 +0000 Subject: [PATCH 1/3] ci: Enforce -Werror, add float-scalar test and sanitizer job Enable -Wall -Wextra -Wdouble-promotion -Werror in CI Bazel builds so regressions are caught immediately. Add float_scalar_test that instantiates L-BFGS, BFGS, gradient descent, conjugate gradient, and Newton with float ScalarType. This exercises code paths that only trigger double-promotion or narrowing when the scalar is not double. Add a separate sanitizer CI job (ASAN + UBSAN) to catch memory errors and undefined behavior. Fix remaining double-promotion sites in lbfgsb.h and armijo.h. Fix armijo.h passing Eigen expression templates where VectorType is required (call .eval() on the expression). Fix unused-variable warnings in test files. Replace deprecated TYPED_TEST_CASE with TYPED_TEST_SUITE. Add [[maybe_unused]] annotations where if-constexpr branches leave parameters unused. --- .github/workflows/ci.yml | 42 ++++++++++--- BUILD | 12 +++- include/cppoptlib/function_base.h | 5 +- include/cppoptlib/function_expressions.h | 30 ++++++---- include/cppoptlib/linesearch/armijo.h | 12 ++-- include/cppoptlib/solver/lbfgsb.h | 4 +- include/cppoptlib/utils/derivatives.h | 8 +-- src/examples/linear_regression.cc | 2 + src/test/augmented_lagrangian_test.cc | 14 ++++- src/test/float_scalar_test.cc | 75 ++++++++++++++++++++++++ src/test/trust_region_newton_test.cc | 5 ++ src/test/verify.cc | 19 +++--- 12 files changed, 183 insertions(+), 45 deletions(-) create mode 100644 src/test/float_scalar_test.cc diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 928443c..b581c76 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,7 +32,6 @@ jobs: libeigen3-dev \ libgtest-dev - - name: Set up Bazel 9 uses: bazel-contrib/setup-bazel@0.19.0 with: @@ -54,16 +53,17 @@ jobs: reporter: github-pr-review flags: '--extensions=h,hpp,c,cpp,cc,cu,hh,ipp --filter=-build/include_order,-whitespace/indent_namespace --recursive include' - - name: Verify + - name: Build (strict warnings) run: | - bazel run verify + bazel build //... \ + --copt=-Wall --copt=-Wextra --copt=-Wpedantic \ + --copt=-Wdouble-promotion --copt=-Werror - - name: Build + - name: Test run: | - bazel run simple - bazel run constrained_simple - bazel run constrained_simple2 - bazel run debug + bazel test //... \ + --copt=-Wall --copt=-Wextra --copt=-Wpedantic \ + --copt=-Wdouble-promotion --copt=-Werror - name: Configure and Build with CMake run: | @@ -72,3 +72,29 @@ jobs: cmake .. -DCMAKE_BUILD_TYPE=Release cmake --build . ctest --output-on-failure + + sanitizers: + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Install System Dependencies + run: | + sudo apt-get update + sudo apt-get install -y g++ libeigen3-dev + + - name: Set up Bazel 9 + uses: bazel-contrib/setup-bazel@0.19.0 + with: + bazelisk-cache: true + disk-cache: ${{ github.workflow }}-san + repository-cache: true + + - name: Test with ASAN + UBSAN + run: | + bazel test //... \ + --copt=-fsanitize=address,undefined \ + --copt=-fno-omit-frame-pointer \ + --linkopt=-fsanitize=address,undefined diff --git a/BUILD b/BUILD index 85365b6..657e7c7 100644 --- a/BUILD +++ b/BUILD @@ -1,5 +1,5 @@ load("//:generator.bzl", "build_example", "build_test") -load("@rules_cc//cc:defs.bzl", "cc_library") +load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test") # Shared header library used by the SVM examples. Exposes the # embedded Iris versicolor-vs-virginica dataset and a classification @@ -23,6 +23,16 @@ build_test("hager_zhang_test") build_test("augmented_lagrangian_test") build_test("trust_region_newton_test") +cc_test( + name = "float_scalar_test", + srcs = ["src/test/float_scalar_test.cc"], + copts = ["-std=c++17", "-Wall", "-Wextra", "-Wdouble-promotion", "-Werror"], + deps = [ + "//include:cppoptlib", + "@eigen//:eigen", + ], +) + build_example("svm_primal_lbfgs", extra_deps = [":iris_data"]) build_example("svm_primal_al", extra_deps = [":iris_data"]) build_example("svm_dual_lbfgsb", extra_deps = [":iris_data"]) diff --git a/include/cppoptlib/function_base.h b/include/cppoptlib/function_base.h index b5a7280..83eb578 100644 --- a/include/cppoptlib/function_base.h +++ b/include/cppoptlib/function_base.h @@ -100,8 +100,9 @@ struct FunctionCRTP : public FunctionInterface { using MatrixType = Eigen::Matrix; static constexpr DifferentiabilityMode Differentiability = TMode; - virtual TScalar operator()(const VectorType& x, VectorType* grad = nullptr, - MatrixType* hess = nullptr) const override { + virtual TScalar operator()( + const VectorType& x, [[maybe_unused]] VectorType* grad = nullptr, + [[maybe_unused]] MatrixType* hess = nullptr) const override { if constexpr (TMode == DifferentiabilityMode::None) { return static_cast(this)->operator()(x); } else if constexpr (TMode == DifferentiabilityMode::First) { diff --git a/include/cppoptlib/function_expressions.h b/include/cppoptlib/function_expressions.h index 9d65c10..6052a8e 100644 --- a/include/cppoptlib/function_expressions.h +++ b/include/cppoptlib/function_expressions.h @@ -54,8 +54,9 @@ struct ConstExpression : public FunctionInterface { TScalar c; explicit ConstExpression(TScalar c_) : c(c_) {} - virtual TScalar operator()(const VectorType& x, VectorType* grad = nullptr, - MatrixType* hess = nullptr) const override { + virtual TScalar operator()( + const VectorType& x, VectorType* grad = nullptr, + [[maybe_unused]] MatrixType* hess = nullptr) const override { (void)x; if (grad) { *grad = VectorType::Zero(x.size()); @@ -164,8 +165,9 @@ struct SubExpression F f; G g; SubExpression(const F& f_, const G& g_) : f(f_), g(g_) {} - virtual ScalarType operator()(const VectorType& x, VectorType* grad = nullptr, - MatrixType* hess = nullptr) const override { + virtual ScalarType operator()( + const VectorType& x, VectorType* grad = nullptr, + [[maybe_unused]] MatrixType* hess = nullptr) const override { if constexpr (TMode == DifferentiabilityMode::None) { return f(x) - g(x); } else if constexpr (TMode == DifferentiabilityMode::First) { @@ -214,8 +216,9 @@ struct MulExpression : public FunctionInterface { TScalar c; F f; MulExpression(const TScalar& c_, const F& f_) : c(c_), f(f_) {} - virtual ScalarType operator()(const VectorType& x, VectorType* grad = nullptr, - MatrixType* hess = nullptr) const override { + virtual ScalarType operator()( + const VectorType& x, VectorType* grad = nullptr, + [[maybe_unused]] MatrixType* hess = nullptr) const override { if (c == TScalar(0)) { if (grad) { *grad = VectorType::Zero(x.size()); @@ -278,8 +281,9 @@ struct ProdExpression ProdExpression(const F& f_, const G& g_) : f(f_), g(g_) {} - virtual ScalarType operator()(const VectorType& x, VectorType* grad = nullptr, - MatrixType* hess = nullptr) const override { + virtual ScalarType operator()( + const VectorType& x, VectorType* grad = nullptr, + [[maybe_unused]] MatrixType* hess = nullptr) const override { if constexpr (TMode == DifferentiabilityMode::None) { return f(x) * g(x); } else if constexpr (TMode == DifferentiabilityMode::First) { @@ -329,8 +333,9 @@ struct MinZeroExpression F f; explicit MinZeroExpression(const F& f_) : f(f_) {} - virtual ScalarType operator()(const VectorType& x, VectorType* grad = nullptr, - MatrixType* hess = nullptr) const override { + virtual ScalarType operator()( + const VectorType& x, VectorType* grad = nullptr, + [[maybe_unused]] MatrixType* hess = nullptr) const override { if constexpr (Differentiability == DifferentiabilityMode::None) { ScalarType val = f(x); // Use ConstExpression to return zero (with zero derivatives) if inactive. @@ -372,8 +377,9 @@ struct MaxZeroExpression F f; explicit MaxZeroExpression(const F& f_) : f(f_) {} - virtual ScalarType operator()(const VectorType& x, VectorType* grad = nullptr, - MatrixType* hess = nullptr) const override { + virtual ScalarType operator()( + const VectorType& x, VectorType* grad = nullptr, + [[maybe_unused]] MatrixType* hess = nullptr) const override { if constexpr (Differentiability == DifferentiabilityMode::None) { ScalarType val = f(x); return (val <= 0) diff --git a/include/cppoptlib/linesearch/armijo.h b/include/cppoptlib/linesearch/armijo.h index 39c3a2a..ec81d7d 100644 --- a/include/cppoptlib/linesearch/armijo.h +++ b/include/cppoptlib/linesearch/armijo.h @@ -45,19 +45,19 @@ class Armijo { static ScalarType Search(const VectorType& x, const VectorType& search_direction, const FunctionType& function, - const ScalarType alpha_init = 1.0) { + const ScalarType alpha_init = ScalarType(1)) { constexpr ScalarType c = ScalarType(0.2); constexpr ScalarType rho = ScalarType(0.9); ScalarType alpha = alpha_init; VectorType gradient; const ScalarType f_in = function(x, &gradient); - ScalarType f = function(x + alpha * search_direction); + ScalarType f = function((x + alpha * search_direction).eval()); const ScalarType cache = c * gradient.dot(search_direction); constexpr ScalarType alpha_min = ScalarType(1e-8); while (f > f_in + alpha * cache && alpha > alpha_min) { alpha *= rho; - f = function(x + alpha * search_direction); + f = function((x + alpha * search_direction).eval()); } return alpha; @@ -84,11 +84,11 @@ class Armijo { const FunctionType& function) { constexpr ScalarType c = ScalarType(0.2); constexpr ScalarType rho = ScalarType(0.9); - ScalarType alpha = 1.0; + ScalarType alpha = ScalarType(1); VectorType gradient; MatrixType hessian; const ScalarType f_in = function(x, &gradient, &hessian); - ScalarType f = function(x + alpha * search_direction); + ScalarType f = function((x + alpha * search_direction).eval()); const ScalarType cache = c * gradient.dot(search_direction) + ScalarType(0.5) * c * c * search_direction.transpose() * @@ -96,7 +96,7 @@ class Armijo { while (f > f_in + alpha * cache) { alpha *= rho; - f = function(x + alpha * search_direction); + f = function((x + alpha * search_direction).eval()); } return alpha; } diff --git a/include/cppoptlib/solver/lbfgsb.h b/include/cppoptlib/solver/lbfgsb.h index e9ebf58..4a03f0b 100644 --- a/include/cppoptlib/solver/lbfgsb.h +++ b/include/cppoptlib/solver/lbfgsb.h @@ -208,7 +208,7 @@ class Lbfgsb // STEP 6: Only update if positive curvature (s'*y > 0) const ScalarType sTy = new_s.dot(new_y); - if (sTy > 1e-7 * new_y.squaredNorm()) { + if (sTy > ScalarType(1e-7) * new_y.squaredNorm()) { if (y_history_.cols() < m) { y_history_.conservativeResize(dim_, y_history_.cols() + 1); s_history_.conservativeResize(dim_, s_history_.cols() + 1); @@ -439,7 +439,7 @@ class Lbfgsb assert(du.rows() == n); for (unsigned int i = 0; i < n; i++) { - if (std::abs(du(i)) < 1e-7) { + if (std::abs(du(i)) < ScalarType(1e-7)) { continue; } else if (du(i) > 0) { alphastar = std::min( diff --git a/include/cppoptlib/utils/derivatives.h b/include/cppoptlib/utils/derivatives.h index 3883cf4..a95e8bf 100644 --- a/include/cppoptlib/utils/derivatives.h +++ b/include/cppoptlib/utils/derivatives.h @@ -255,12 +255,12 @@ template bool IsGradientCorrect(const FunctionType& function, const typename FunctionType::VectorType& x0, int accuracy = 3) { - constexpr float tolerance = 1e-2; - using ScalarType = typename FunctionType::ScalarType; using VectorType = typename FunctionType::VectorType; using index_t = typename VectorType::Index; + constexpr ScalarType tolerance = ScalarType(1e-2); + const index_t D = x0.rows(); VectorType actual_gradient; function(x0, &actual_gradient); @@ -283,13 +283,13 @@ template bool IsHessianCorrect(const FunctionType& function, const typename FunctionType::VectorType& x0, int accuracy = 3) { - constexpr float tolerance = 1e-1; - using ScalarType = typename FunctionType::ScalarType; using MatrixType = typename FunctionType::MatrixType; using VectorType = typename FunctionType::VectorType; using index_t = typename VectorType::Index; + constexpr ScalarType tolerance = ScalarType(1e-1); + const index_t D = x0.rows(); MatrixType actual_hessian; diff --git a/src/examples/linear_regression.cc b/src/examples/linear_regression.cc index 6081acb..58841ed 100644 --- a/src/examples/linear_regression.cc +++ b/src/examples/linear_regression.cc @@ -72,6 +72,7 @@ int main() { const auto initial_state = cppoptlib::function::FunctionState(x); auto [solution, solver_state] = solver.Minimize(f, initial_state); + (void)solver_state; std::cout << "argmin " << solution.x.transpose() << std::endl; // Or model it as a augmented Lagrangian @@ -97,6 +98,7 @@ int main() { // Run the agumented solver. auto [aug_solution, aug_solver_state] = aug_solver.Minimize(l_state); std::cout << "argmin " << aug_solution.x.transpose() << std::endl; + (void)aug_solver_state; return 0; } diff --git a/src/test/augmented_lagrangian_test.cc b/src/test/augmented_lagrangian_test.cc index 2fe81d7..dd3b13f 100644 --- a/src/test/augmented_lagrangian_test.cc +++ b/src/test/augmented_lagrangian_test.cc @@ -507,6 +507,7 @@ TEST(AugmentedLagrangianKKT, EqualityOnlyQuadratic) { x0 << 5.0, 5.0; cppoptlib::solver::AugmentedLagrangeState state(x0, 1, 0, 1.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; // Primal optimum. EXPECT_NEAR(1.0, solution.x[0], kkt_primal_tolerance); @@ -558,6 +559,7 @@ TEST(AugmentedLagrangianKKT, InequalityActiveRecoversMultiplier) { x0 << 5.0, 5.0; cppoptlib::solver::AugmentedLagrangeState state(x0, 0, 1, 1.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; EXPECT_NEAR(1.0, solution.x[0], kkt_primal_tolerance); EXPECT_NEAR(0.0, solution.x[1], kkt_primal_tolerance); @@ -601,6 +603,7 @@ TEST(AugmentedLagrangianKKT, BothEqualityAndInequalityActive) { x0 << 1.0, 1.0; cppoptlib::solver::AugmentedLagrangeState state(x0, 1, 1, 1.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; EXPECT_NEAR(0.5, solution.x[0], kkt_primal_tolerance); EXPECT_NEAR(1.5, solution.x[1], kkt_primal_tolerance); @@ -643,6 +646,7 @@ TEST(AugmentedLagrangianOuter, FeasibleStartConvergesImmediately) { x0 << 0.0, 0.0; cppoptlib::solver::AugmentedLagrangeState state(x0, 1, 0, 1.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; EXPECT_NEAR(0.0, solution.x[0], kkt_primal_tolerance); EXPECT_NEAR(0.0, solution.x[1], kkt_primal_tolerance); @@ -676,6 +680,7 @@ TEST(AugmentedLagrangianOuter, NoConstraintsIsUnconstrained) { x0 << 5.0, 5.0; cppoptlib::solver::AugmentedLagrangeState state(x0, 0, 0, 1.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; EXPECT_NEAR(0.0, solution.x[0], kkt_primal_tolerance); EXPECT_NEAR(0.0, solution.x[1], kkt_primal_tolerance); @@ -711,6 +716,7 @@ TEST(AugmentedLagrangianOuter, PenaltyHoldsFlatOnFeasibleProblem) { cppoptlib::solver::AugmentedLagrangeState state(x0, 1, 0, initial_penalty); auto [solution, progress] = solver.Minimize(state); + (void)progress; // The conditional schedule should never fire on a trivially feasible // problem. The penalty is pinned exactly at its initial value. @@ -748,6 +754,7 @@ TEST(AugmentedLagrangianOuter, PenaltyGrowthCanBeDisabled) { cppoptlib::solver::AugmentedLagrangeState state(x0, 1, 0, initial_penalty); auto [solution, progress] = solver.Minimize(state); + (void)progress; EXPECT_EQ(initial_penalty, solution.penalty_state.penalty); } @@ -781,6 +788,7 @@ TEST(AugmentedLagrangianOuter, PenaltyGrowsOnlyWhileViolationLags) { x0 << 5.0, 5.0; cppoptlib::solver::AugmentedLagrangeState state(x0, 1, 0, 1.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; constexpr double penalty_upper_bound = 1e4; EXPECT_LE(solution.penalty_state.penalty, penalty_upper_bound); @@ -1051,6 +1059,7 @@ TEST(AugmentedLagrangianNonConvex, Hs024TriangleEscapesSpuriousOrigin) { cppoptlib::solver::AugmentedLagrangeState state( x0, /*num_eq=*/0, /*num_ineq=*/3, /*penalty=*/0.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; const double f_final = objective(solution.x); @@ -1124,6 +1133,7 @@ TEST(AugmentedLagrangianNonConvex, Hs029EllipseEscapesOrigin) { cppoptlib::solver::AugmentedLagrangeState state( x0, /*num_eq=*/0, /*num_ineq=*/1, /*penalty=*/0.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; const double f_final = objective(solution.x); // True optimum for `min -x0*x1 s.t. 48 - x0^2 - 2 x1^2 >= 0` is at @@ -1171,6 +1181,7 @@ TEST(AugmentedLagrangianOuter, KktStationarityReportedOnFinishedState) { x0 << 5.0, 5.0; cppoptlib::solver::AugmentedLagrangeState state(x0, 1, 0, 1.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; ASSERT_EQ(cppoptlib::solver::Status::Finished, progress.status); // The outer-loop KKT tolerance defaults match the primal one to a @@ -1197,7 +1208,7 @@ TEST(AugmentedLagrangianOuter, KktStationarityReportedOnFinishedState) { // `progress.status == Finished` fails. TEST(AugmentedLagrangianBoxInterface, BoxPinnedOptimumStopsOnKkt) { using cppoptlib::function::FunctionExpr; - using VectorType = Eigen::Matrix; + using VectorType [[maybe_unused]] = Eigen::Matrix; class Rosenbrock : public cppoptlib::function::FunctionCRTP< Rosenbrock, double, @@ -1261,6 +1272,7 @@ TEST(AugmentedLagrangianBoxInterface, BoxPinnedOptimumStopsOnKkt) { cppoptlib::solver::AugmentedLagrangeState state(x0, 0, 2, 0.0); auto [solution, progress] = solver.Minimize(state); + (void)progress; // The outer loop must reach `Finished` status on its own -- an // `IterationLimit` exit would signal regression of the projected- diff --git a/src/test/float_scalar_test.cc b/src/test/float_scalar_test.cc new file mode 100644 index 0000000..9a62004 --- /dev/null +++ b/src/test/float_scalar_test.cc @@ -0,0 +1,75 @@ +// Instantiates all solvers with float to catch double-promotion and +// narrowing issues that only surface with non-double scalar types. +// The goal is exercising template instantiation, not convergence testing. + +#include +#include +#include + +#include "Eigen/Core" +#include "cppoptlib/function.h" +#include "cppoptlib/solver/bfgs.h" +#include "cppoptlib/solver/conjugated_gradient_descent.h" +#include "cppoptlib/solver/gradient_descent.h" +#include "cppoptlib/solver/lbfgs.h" +#include "cppoptlib/solver/newton_descent.h" + +using FunctionExprF1 = cppoptlib::function::FunctionExpr< + float, cppoptlib::function::DifferentiabilityMode::First, 2>; +using FunctionExprF2 = cppoptlib::function::FunctionExpr< + float, cppoptlib::function::DifferentiabilityMode::Second, 2>; + +class FloatQuadratic1 + : public cppoptlib::function::FunctionCRTP< + FloatQuadratic1, float, + cppoptlib::function::DifferentiabilityMode::First, 2> { + public: + ScalarType operator()(const VectorType& x, VectorType* grad) const { + if (grad) *grad = Eigen::Vector2f(10 * x[0], 200 * x[1]); + return 5 * x[0] * x[0] + 100 * x[1] * x[1] + 5; + } +}; + +class FloatQuadratic2 + : public cppoptlib::function::FunctionCRTP< + FloatQuadratic2, float, + cppoptlib::function::DifferentiabilityMode::Second, 2> { + public: + ScalarType operator()(const VectorType& x, VectorType* grad, + MatrixType* hess) const { + if (grad) *grad = Eigen::Vector2f(10 * x[0], 200 * x[1]); + if (hess) { + hess->setZero(); + hess->diagonal() << 10, 200; + } + return 5 * x[0] * x[0] + 100 * x[1] * x[1] + 5; + } +}; + +template +void RunSolver(typename Solver::FunctionType f) { + Eigen::Vector2f x0(-1.0f, 0.5f); + Solver solver; + auto [solution, solver_state] = + solver.Minimize(f, cppoptlib::function::FunctionState(x0)); + // Verify the solver ran and produced a finite result. + assert(solver_state.num_iterations > 0); + assert(std::isfinite(solution.value)); + // Strong solvers must converge on this simple problem. + (void)solution; +} + +int main() { + RunSolver>( + FunctionExprF1(FloatQuadratic1())); + RunSolver>( + FunctionExprF2(FloatQuadratic2())); + RunSolver>( + FunctionExprF1(FloatQuadratic1())); + RunSolver>( + FunctionExprF1(FloatQuadratic1())); + RunSolver>( + FunctionExprF2(FloatQuadratic2())); + std::cout << "PASS: all float-scalar solvers ran successfully\n"; + return 0; +} diff --git a/src/test/trust_region_newton_test.cc b/src/test/trust_region_newton_test.cc index ce29497..4469d32 100644 --- a/src/test/trust_region_newton_test.cc +++ b/src/test/trust_region_newton_test.cc @@ -206,6 +206,8 @@ TEST(TrustRegionNewton, TrustRegionBoundaryExitRespectsRadius) { } }); auto [solution, progress] = solver.Minimize(f, FunctionState(x0)); + (void)solution; + (void)progress; const double first_step_norm = (x_after_first_step - x0).norm(); EXPECT_NEAR(first_step_norm, config.initial_radius, 1e-10); @@ -234,6 +236,7 @@ TEST(TrustRegionNewton, IndefiniteHessianNegativeCurvatureStepIsBounded) { } }); auto [solution, progress] = solver.Minimize(f, FunctionState(x0)); + (void)progress; const double first_step_norm = (x_after_first_step - x0).norm(); EXPECT_LE(first_step_norm, config.initial_radius + 1e-10); @@ -339,6 +342,7 @@ TEST(TrustRegionNewton, GradientNormStopFires) { Eigen::Vector2d x0(3.0, 3.0); auto [solution, progress] = solver.Minimize(f, FunctionState(x0)); + (void)solution; // A regression that disabled the gradient test would hit the // 100-iteration cap and return `IterationLimit`. We want to see @@ -359,6 +363,7 @@ TEST(TrustRegionNewton, IterationLimitStopFires) { Eigen::Vector2d x0(-1.2, 1.0); auto [solution, progress] = solver.Minimize(f, FunctionState(x0)); + (void)solution; EXPECT_EQ(progress.status, cppoptlib::solver::Status::IterationLimit); } diff --git a/src/test/verify.cc b/src/test/verify.cc index f9021dd..3b94d58 100644 --- a/src/test/verify.cc +++ b/src/test/verify.cc @@ -155,15 +155,15 @@ class NelderMeadTest : public testing::Test {}; EXPECT_NEAR(fx, f(solution.x), PRECISION); typedef ::testing::Types DoublePrecision; -TYPED_TEST_CASE(GradientDescentTest, DoublePrecision); -TYPED_TEST_CASE(ConjugatedGradientDescentTest, DoublePrecision); -TYPED_TEST_CASE(NewtonDescentTest, DoublePrecision); -TYPED_TEST_CASE(BfgsTest, DoublePrecision); -TYPED_TEST_CASE(LbfgsTest, DoublePrecision); +TYPED_TEST_SUITE(GradientDescentTest, DoublePrecision); +TYPED_TEST_SUITE(ConjugatedGradientDescentTest, DoublePrecision); +TYPED_TEST_SUITE(NewtonDescentTest, DoublePrecision); +TYPED_TEST_SUITE(BfgsTest, DoublePrecision); +TYPED_TEST_SUITE(LbfgsTest, DoublePrecision); #if EIGEN_VERSION_AT_LEAST(3, 4, 0) -TYPED_TEST_CASE(LbfgsbTest, DoublePrecision); +TYPED_TEST_SUITE(LbfgsbTest, DoublePrecision); #endif -TYPED_TEST_CASE(NelderMeadTest, DoublePrecision); +TYPED_TEST_SUITE(NelderMeadTest, DoublePrecision); #define SOLVER_SETUP(sol, func) \ TYPED_TEST(sol##Test, func##Far){SOLVE_PROBLEM( \ @@ -208,7 +208,7 @@ class SimpleFunction : public FunctionX2> { template class CentralDifference : public testing::Test {}; -TYPED_TEST_CASE(CentralDifference, DoublePrecision); +TYPED_TEST_SUITE(CentralDifference, DoublePrecision); TYPED_TEST(CentralDifference, Gradient) { typename SimpleFunction::VectorType x0(2); @@ -281,7 +281,7 @@ class Circle : public Function2d { template class Constrained : public testing::Test {}; -TYPED_TEST_CASE(Constrained, DoublePrecision); +TYPED_TEST_SUITE(Constrained, DoublePrecision); TYPED_TEST(Constrained, Simple) { constexpr auto dim = 2; SumObjective::VectorType x(dim); @@ -307,6 +307,7 @@ TYPED_TEST(Constrained, Simple) { // Run the solver. auto [solution, solver_state] = solver.Minimize(l_state); + (void)solver_state; EXPECT_NEAR(solution.x[0], -1, 1e-3); EXPECT_NEAR(solution.x[1], -1, 1e-3); } From 36e97f6e8f799adc16332aca56d6f15bc6f69dc5 Mon Sep 17 00:00:00 2001 From: patwie Date: Sun, 28 Jun 2026 23:19:26 +0000 Subject: [PATCH 2/3] test: Fix -Werror -Wpedantic failures in test suite The -Werror CI build failed on two fronts that the strict-warnings commit did not cover. First, verify.cc invoked gtest's variadic TYPED_TEST_SUITE(CaseName, Types, ...) with only two arguments. Under -Wpedantic GCC rejects the empty trailing __VA_ARGS__ as a GNU extension ("ISO C++11 requires at least one argument for the ... in a variadic macro"). Rather than suppress the diagnostic, supply the macro's documented optional third argument: a type-name generator. That makes __VA_ARGS__ non-empty so the invocation is strictly conforming, and names each typed-test instantiation after its scalar type instead of a bare index. Second, the two Dynamic-dimension trust-region tests bound a `progress` result they never read, tripping -Wunused-variable. Instead of voiding it, assert on it: the solver must converge within its iteration budget rather than exhaust it. Which convergence criterion fires depends on Hessian conditioning -- the 2-D case trips the gradient-norm test, the worse-conditioned 5-D case trips the x-delta test first -- so the assertion checks the stable invariant (status is not IterationLimit) rather than a specific success status. Verified with `bazel test //... --copt=-Wall --copt=-Wextra --copt=-Wpedantic --copt=-Wdouble-promotion --copt=-Werror` (all green) and under the ASAN+UBSAN sanitizer flags. --- src/test/trust_region_newton_test.cc | 11 ++++++++ src/test/verify.cc | 40 +++++++++++++++++++++------- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/test/trust_region_newton_test.cc b/src/test/trust_region_newton_test.cc index 4469d32..389e6cd 100644 --- a/src/test/trust_region_newton_test.cc +++ b/src/test/trust_region_newton_test.cc @@ -410,6 +410,15 @@ TEST(TrustRegionNewton, DynamicDimensionStrictlyConvexConverges) { ASSERT_EQ(solution.x.size(), 2); EXPECT_NEAR(solution.x(0), 0.0, 1e-8); EXPECT_NEAR(solution.x(1), 0.0, 1e-8); + // The dynamic path must converge within its iteration budget rather + // than stall or exhaust it. Which convergence criterion fires + // (gradient-norm vs. x-delta) depends on Hessian conditioning, so we + // assert the stable invariant -- not `IterationLimit` -- instead of a + // specific success status. Before the dim_ fix this test never + // reached this assertion: it crashed inside the CG-Steihaug + // subproblem solver on a size-0 step vector. + EXPECT_NE(progress.status, cppoptlib::solver::Status::IterationLimit); + EXPECT_LT(progress.num_iterations, static_cast(50)); } TEST(TrustRegionNewton, DynamicDimensionHigherDimConverges) { @@ -424,6 +433,8 @@ TEST(TrustRegionNewton, DynamicDimensionHigherDimConverges) { ASSERT_EQ(solution.x.size(), 5); EXPECT_LT(solution.x.norm(), 1e-6); + EXPECT_NE(progress.status, cppoptlib::solver::Status::IterationLimit); + EXPECT_LT(progress.num_iterations, static_cast(100)); } int main(int argc, char** argv) { diff --git a/src/test/verify.cc b/src/test/verify.cc index 3b94d58..aa6612d 100644 --- a/src/test/verify.cc +++ b/src/test/verify.cc @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include "cppoptlib/function.h" #include "cppoptlib/solver/augmented_lagrangian.h" @@ -155,15 +157,35 @@ class NelderMeadTest : public testing::Test {}; EXPECT_NEAR(fx, f(solution.x), PRECISION); typedef ::testing::Types DoublePrecision; -TYPED_TEST_SUITE(GradientDescentTest, DoublePrecision); -TYPED_TEST_SUITE(ConjugatedGradientDescentTest, DoublePrecision); -TYPED_TEST_SUITE(NewtonDescentTest, DoublePrecision); -TYPED_TEST_SUITE(BfgsTest, DoublePrecision); -TYPED_TEST_SUITE(LbfgsTest, DoublePrecision); + +// gtest's TYPED_TEST_SUITE takes an optional third argument: a type-name +// generator (a class with a static template `GetName(int)`). We supply +// one explicitly rather than relying on the default. Doing so makes the +// macro's trailing `__VA_ARGS__` non-empty, which keeps the invocation +// strictly ISO C++ conforming under `-Wpedantic` -- an empty variadic +// macro argument is a GNU extension that `-Wpedantic -Werror` rejects. +// As a bonus the generator gives each typed-test instantiation a readable +// scalar-type suffix instead of a bare numeric index. +class ScalarTypeName { + public: + template + static std::string GetName(int) { + if (std::is_same::value) return "double"; + if (std::is_same::value) return "float"; + return "scalar"; + } +}; + +TYPED_TEST_SUITE(GradientDescentTest, DoublePrecision, ScalarTypeName); +TYPED_TEST_SUITE(ConjugatedGradientDescentTest, DoublePrecision, + ScalarTypeName); +TYPED_TEST_SUITE(NewtonDescentTest, DoublePrecision, ScalarTypeName); +TYPED_TEST_SUITE(BfgsTest, DoublePrecision, ScalarTypeName); +TYPED_TEST_SUITE(LbfgsTest, DoublePrecision, ScalarTypeName); #if EIGEN_VERSION_AT_LEAST(3, 4, 0) -TYPED_TEST_SUITE(LbfgsbTest, DoublePrecision); +TYPED_TEST_SUITE(LbfgsbTest, DoublePrecision, ScalarTypeName); #endif -TYPED_TEST_SUITE(NelderMeadTest, DoublePrecision); +TYPED_TEST_SUITE(NelderMeadTest, DoublePrecision, ScalarTypeName); #define SOLVER_SETUP(sol, func) \ TYPED_TEST(sol##Test, func##Far){SOLVE_PROBLEM( \ @@ -208,7 +230,7 @@ class SimpleFunction : public FunctionX2> { template class CentralDifference : public testing::Test {}; -TYPED_TEST_SUITE(CentralDifference, DoublePrecision); +TYPED_TEST_SUITE(CentralDifference, DoublePrecision, ScalarTypeName); TYPED_TEST(CentralDifference, Gradient) { typename SimpleFunction::VectorType x0(2); @@ -281,7 +303,7 @@ class Circle : public Function2d { template class Constrained : public testing::Test {}; -TYPED_TEST_SUITE(Constrained, DoublePrecision); +TYPED_TEST_SUITE(Constrained, DoublePrecision, ScalarTypeName); TYPED_TEST(Constrained, Simple) { constexpr auto dim = 2; SumObjective::VectorType x(dim); From 86f3ee9c1e25c2d674f712867f05bf68a76c70fe Mon Sep 17 00:00:00 2001 From: patwie Date: Sun, 28 Jun 2026 23:28:28 +0000 Subject: [PATCH 3/3] Update macOS presubmit platform to ARM64 Replace the generic macOS platform identifier with the more specific macOS ARM64 variant in the Bazel Central Registry presubmit matrix. This ensures CI testing runs on the correct macOS architecture, improving compatibility verification for ARM-based macOS systems. --- .bcr/presubmit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bcr/presubmit.yml b/.bcr/presubmit.yml index c02a934..bd0ff01 100644 --- a/.bcr/presubmit.yml +++ b/.bcr/presubmit.yml @@ -2,7 +2,7 @@ matrix: platform: - debian11 - ubuntu2004 - - macos + - macos_arm64 bazel: - 7.x - 8.x