From 0df3cd98dce408d134cd47881f9696dc64ea9ef1 Mon Sep 17 00:00:00 2001 From: shakedregev Date: Mon, 5 Oct 2026 14:14:10 -0400 Subject: [PATCH 1/6] fixed sloppy warnings and changelog --- CHANGELOG.md | 5 +++++ resolve/SystemSolver.cpp | 48 ++++++++++++++++------------------------ 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05da4c87b..0df4db9f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Re::Solve Changelog +## Changes to Re::Solve since release 0.99.4 + +- Refactored system solver: Added helper functions to check and fix conflicts in configuration options and to reduce code duplication. +- Code throws errors instead of warnings when encountering ambiguous erronneus inputs from the user. + ## HyKKT Release changes - Exported HyKKT libraries and fixed solver reuse by refreshing numerical data, reusing allocations, resizing GPU transpose workspaces, and handling zero residuals. diff --git a/resolve/SystemSolver.cpp b/resolve/SystemSolver.cpp index b5b5b381d..390fc4444 100644 --- a/resolve/SystemSolver.cpp +++ b/resolve/SystemSolver.cpp @@ -53,8 +53,8 @@ namespace ReSolve // LinSolverIterativeRandFGMRES) as public static methods. namespace { - /// Maps string ID to the sketching method enum, warns and defaults to count sketch. - LinSolverIterativeRandFGMRES::SketchingMethod sketchingMethodFromString(const std::string& method) + /// Maps string ID to the sketching method enum. Throws an error if the method is not recognized. + LinSolverIterativeRandFGMRES::SketchingMethod sketchingMethodFromString(const std::string& method = "count") { if (method == "count") { @@ -64,13 +64,12 @@ namespace ReSolve { return LinSolverIterativeRandFGMRES::fwht; } - out::warning() << "Sketching method " << method << " not recognized!\n" - << "Using default (count sketch).\n"; + out::error() << "Sketching method " << method << " not recognized! Valid arguments are \"count\" and \"fwht\".\n"; return LinSolverIterativeRandFGMRES::cs; } - /// Maps string ID to the Gram-Schmidt variant enum, warns and defaults to CGS2. - GramSchmidt::GSVariant gsVariantFromString(const std::string& variant) + /// Maps string ID to the Gram-Schmidt variant enum. Defaults to CGS2. + GramSchmidt::GSVariant gsVariantFromString(const std::string& variant = "cgs2") { if (variant == "cgs2") { @@ -92,8 +91,7 @@ namespace ReSolve { return GramSchmidt::CGS1; } - out::warning() << "Gram-Schmidt variant " << variant << " not recognized.\n"; - out::warning() << "Using default CGS2 Gram-Schmidt variant.\n"; + out::error() << "Gram-Schmidt variant " << variant << " not recognized! Valid arguments are \"cgs2\", \"mgs\", \"mgs_two_sync\", \"mgs_pm\", and \"cgs1\".\n"; return GramSchmidt::CGS2; } @@ -750,10 +748,8 @@ namespace ReSolve { if (method != "none") { - out::warning() << "Iterative refinement cannot be enabled together with an " - << "iterative solve method ('randgmres' or 'fgmres'). " - << "Keeping refinement method 'none'.\n"; - ir_method_ = "none"; + out::error() << "Iterative refinement cannot be enabled together with an " + << "iterative solve method ('randgmres' or 'fgmres').\n"; return 1; } ir_method_ = "none"; @@ -964,8 +960,7 @@ namespace ReSolve { if (solve_method_ != "randgmres") { - out::warning() << "Trying to set sketching method to an incompatible solver.\n"; - out::warning() << "The setting will be ignored.\n"; + out::error() << "Trying to set sketching method to an incompatible solver. Sketching can only be used with the \"randgmres\" solver.\n"; return 1; } @@ -994,8 +989,7 @@ namespace ReSolve * * Records the variant in `gs_method_` so that it survives re-creation of * the Krylov solver, and applies it to the existing `GramSchmidt` object - * or creates one if none exists yet. An unrecognized string ID falls back - * to CGS2 with a warning, and `gs_method_` is set to "cgs2" accordingly. + * or creates one if none exists yet. * * @param[in] variant - string ID of the Gram-Schmidt variant * @@ -1064,7 +1058,7 @@ namespace ReSolve * refactorization are set, iterative refinement is optional, and the * preconditioner must be "none". * - * Offending settings are reset to "none" with a warning. + * If the user selects incompatible methods, the program will terminate with an error message. */ void SystemSolver::validateConfiguration() { @@ -1074,34 +1068,30 @@ namespace ReSolve { if (factorization_method_ != "none") { - out::warning() << "Incorrect input: factorization method '" << factorization_method_ + out::error() << "Incorrect input: factorization method '" << factorization_method_ << "' cannot be used with iterative solve method '" << solve_method_ - << "'. Setting factorization to 'none' ...\n"; - factorization_method_ = "none"; + << "\n"; } if (refactorization_method_ != "none") { - out::warning() << "Incorrect input: refactorization method '" << refactorization_method_ + out::error() << "Incorrect input: refactorization method '" << refactorization_method_ << "' cannot be used with iterative solve method '" << solve_method_ - << "'. Setting refactorization to 'none' ...\n"; - refactorization_method_ = "none"; + << "\n"; } if (ir_method_ != "none") { - out::warning() << "Incorrect input: iterative refinement cannot be enabled " + out::error() << "Incorrect input: iterative refinement cannot be enabled " << "together with iterative solve method '" << solve_method_ - << "'. Setting refinement method to 'none' ...\n"; - ir_method_ = "none"; + << "\n"; } } else { if (precondition_method_ != "none") { - out::warning() << "Incorrect input: preconditioner '" << precondition_method_ + out::error() << "Incorrect input: preconditioner '" << precondition_method_ << "' can only be used with an iterative solve method ('fgmres' or 'randgmres'). " - << "Setting preconditioner to 'none' ...\n"; - precondition_method_ = "none"; + << "\n"; } } } From 8ac96c8c214332848a8b45f4ac4b35b1fc285f1c Mon Sep 17 00:00:00 2001 From: shakedregev Date: Mon, 5 Oct 2026 18:17:38 +0000 Subject: [PATCH 2/6] Apply pre-commmit fixes --- resolve/SystemSolver.cpp | 18 +++++++++--------- tests/unit/hykkt/HykktSolverTests.hpp | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/resolve/SystemSolver.cpp b/resolve/SystemSolver.cpp index 390fc4444..59e51a688 100644 --- a/resolve/SystemSolver.cpp +++ b/resolve/SystemSolver.cpp @@ -749,7 +749,7 @@ namespace ReSolve if (method != "none") { out::error() << "Iterative refinement cannot be enabled together with an " - << "iterative solve method ('randgmres' or 'fgmres').\n"; + << "iterative solve method ('randgmres' or 'fgmres').\n"; return 1; } ir_method_ = "none"; @@ -1069,20 +1069,20 @@ namespace ReSolve if (factorization_method_ != "none") { out::error() << "Incorrect input: factorization method '" << factorization_method_ - << "' cannot be used with iterative solve method '" << solve_method_ - << "\n"; + << "' cannot be used with iterative solve method '" << solve_method_ + << "\n"; } if (refactorization_method_ != "none") { out::error() << "Incorrect input: refactorization method '" << refactorization_method_ - << "' cannot be used with iterative solve method '" << solve_method_ - << "\n"; + << "' cannot be used with iterative solve method '" << solve_method_ + << "\n"; } if (ir_method_ != "none") { out::error() << "Incorrect input: iterative refinement cannot be enabled " - << "together with iterative solve method '" << solve_method_ - << "\n"; + << "together with iterative solve method '" << solve_method_ + << "\n"; } } else @@ -1090,8 +1090,8 @@ namespace ReSolve if (precondition_method_ != "none") { out::error() << "Incorrect input: preconditioner '" << precondition_method_ - << "' can only be used with an iterative solve method ('fgmres' or 'randgmres'). " - << "\n"; + << "' can only be used with an iterative solve method ('fgmres' or 'randgmres'). " + << "\n"; } } } diff --git a/tests/unit/hykkt/HykktSolverTests.hpp b/tests/unit/hykkt/HykktSolverTests.hpp index 4d07f9ac1..da08c692f 100644 --- a/tests/unit/hykkt/HykktSolverTests.hpp +++ b/tests/unit/hykkt/HykktSolverTests.hpp @@ -215,13 +215,13 @@ namespace ReSolve status *= validateResult(zero_rhs_error, tol); // Check that the solver raises an error when trying to change J_d - // from nonempty to empty. + // from nonempty to empty. matrix::Csr* J_d_empty = new matrix::Csr(J_d->getNumRows(), J_d->getNumColumns(), 0); io::Logger::Verbosity saved_verbosity = io::Logger::verbosity(); - io::Logger::setVerbosity(io::Logger::NONE); //The error is expected, so mute the logger before triggering it. + io::Logger::setVerbosity(io::Logger::NONE); // The error is expected, so mute the logger before triggering it. int structure_status = hykktSolver.setMatrixBlocks(H, D_s_reuse, J, J_d_empty); io::Logger::setVerbosity(saved_verbosity); status *= (structure_status != 0); From f896f1874cfb35f1d34ade528b6057adeecd3791 Mon Sep 17 00:00:00 2001 From: Shaked Regev <35384901+shakedregev@users.noreply.github.com> Date: Mon, 5 Oct 2026 14:20:34 -0400 Subject: [PATCH 3/6] Fixed comments. Co-authored-by: Shaked Regev <35384901+shakedregev@users.noreply.github.com> --- resolve/SystemSolver.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resolve/SystemSolver.cpp b/resolve/SystemSolver.cpp index 59e51a688..48bc5220a 100644 --- a/resolve/SystemSolver.cpp +++ b/resolve/SystemSolver.cpp @@ -53,7 +53,7 @@ namespace ReSolve // LinSolverIterativeRandFGMRES) as public static methods. namespace { - /// Maps string ID to the sketching method enum. Throws an error if the method is not recognized. + /// Maps string ID to the sketching method enum. Throws an error if the user inputs an unrecognized method. LinSolverIterativeRandFGMRES::SketchingMethod sketchingMethodFromString(const std::string& method = "count") { if (method == "count") @@ -68,7 +68,7 @@ namespace ReSolve return LinSolverIterativeRandFGMRES::cs; } - /// Maps string ID to the Gram-Schmidt variant enum. Defaults to CGS2. + /// Maps string ID to the Gram-Schmidt variant enum. Throws an error if the user inputs an unrecognized method. GramSchmidt::GSVariant gsVariantFromString(const std::string& variant = "cgs2") { if (variant == "cgs2") From 9db630a99bea574c69570e0855a637a7415a447f Mon Sep 17 00:00:00 2001 From: Shaked Regev <35384901+shakedregev@users.noreply.github.com> Date: Tue, 6 Oct 2026 10:50:54 -0400 Subject: [PATCH 4/6] Apply batched suggestions from code review Co-authored-by: pelesh --- resolve/SystemSolver.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/resolve/SystemSolver.cpp b/resolve/SystemSolver.cpp index 48bc5220a..a0ea3fa80 100644 --- a/resolve/SystemSolver.cpp +++ b/resolve/SystemSolver.cpp @@ -53,7 +53,7 @@ namespace ReSolve // LinSolverIterativeRandFGMRES) as public static methods. namespace { - /// Maps string ID to the sketching method enum. Throws an error if the user inputs an unrecognized method. + /// Maps string ID to the sketching method enum. Returns an error if the user inputs an unrecognized method. LinSolverIterativeRandFGMRES::SketchingMethod sketchingMethodFromString(const std::string& method = "count") { if (method == "count") @@ -68,7 +68,7 @@ namespace ReSolve return LinSolverIterativeRandFGMRES::cs; } - /// Maps string ID to the Gram-Schmidt variant enum. Throws an error if the user inputs an unrecognized method. + /// Maps string ID to the Gram-Schmidt variant enum. Returns an error if the user inputs an unrecognized method. GramSchmidt::GSVariant gsVariantFromString(const std::string& variant = "cgs2") { if (variant == "cgs2") @@ -91,7 +91,8 @@ namespace ReSolve { return GramSchmidt::CGS1; } - out::error() << "Gram-Schmidt variant " << variant << " not recognized! Valid arguments are \"cgs2\", \"mgs\", \"mgs_two_sync\", \"mgs_pm\", and \"cgs1\".\n"; + out::error() << "Gram-Schmidt variant " << variant << " not recognized! Valid arguments are:\n" + "\"cgs2\", \"mgs\", \"mgs_two_sync\", \"mgs_pm\", and \"cgs1\".\n"; return GramSchmidt::CGS2; } @@ -960,7 +961,8 @@ namespace ReSolve { if (solve_method_ != "randgmres") { - out::error() << "Trying to set sketching method to an incompatible solver. Sketching can only be used with the \"randgmres\" solver.\n"; + out::error() << "Trying to set sketching method to an incompatible solver\n." + "Sketching can only be used with the \"randgmres\" solver.\n"; return 1; } From 51a03c84b1506d1bb31016c6c77e4960ddb39364 Mon Sep 17 00:00:00 2001 From: shakedregev Date: Tue, 6 Oct 2026 14:51:14 +0000 Subject: [PATCH 5/6] Apply pre-commmit fixes --- resolve/SystemSolver.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resolve/SystemSolver.cpp b/resolve/SystemSolver.cpp index a0ea3fa80..b1ab0c826 100644 --- a/resolve/SystemSolver.cpp +++ b/resolve/SystemSolver.cpp @@ -92,7 +92,7 @@ namespace ReSolve return GramSchmidt::CGS1; } out::error() << "Gram-Schmidt variant " << variant << " not recognized! Valid arguments are:\n" - "\"cgs2\", \"mgs\", \"mgs_two_sync\", \"mgs_pm\", and \"cgs1\".\n"; + "\"cgs2\", \"mgs\", \"mgs_two_sync\", \"mgs_pm\", and \"cgs1\".\n"; return GramSchmidt::CGS2; } @@ -962,7 +962,7 @@ namespace ReSolve if (solve_method_ != "randgmres") { out::error() << "Trying to set sketching method to an incompatible solver\n." - "Sketching can only be used with the \"randgmres\" solver.\n"; + "Sketching can only be used with the \"randgmres\" solver.\n"; return 1; } From 63a99cb0ef021a672aa87434c0ebf4fc900a9668 Mon Sep 17 00:00:00 2001 From: Shaked Regev <35384901+shakedregev@users.noreply.github.com> Date: Tue, 6 Oct 2026 10:53:13 -0400 Subject: [PATCH 6/6] Apply suggestion from @shakedregev --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0df4db9f7..72c0c4d7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## Changes to Re::Solve since release 0.99.4 - Refactored system solver: Added helper functions to check and fix conflicts in configuration options and to reduce code duplication. -- Code throws errors instead of warnings when encountering ambiguous erronneus inputs from the user. +- Code returns errors instead of warnings when encountering ambiguous erronneus inputs from the user. ## HyKKT Release changes