From 22f84b1b5856263d23ef7a03f84bfec1ca5e26f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toma=C5=BE=20Vajngerl?= Date: Wed, 3 Jun 2026 09:54:08 +0900 Subject: [PATCH] SwarmSolver: handle an empty or reversed variable range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a variable's lower and upper bound are equal, or when contradictory constraints leave the lower bound above the upper one, the range is empty or reversed. boundVariable kept wrapping an out of range value without the step ever bringing it inside, so solve never returned. The random distributions in initializeVariables and std::clamp in clampVariable are also undefined when the lower bound is not below the upper one. Treat an empty or reversed range as having only the lower bound: return it directly in clampVariable and boundVariable, and seed it as the value in initializeVariables. boundVariable now also wraps a normal out of range value in one step instead of looping. Add a test with a variable bounded both below 5 and above 10. Before the fix solve looped forever. It now returns and reports the model as not solved. Signed-off-by: Tomaž Vajngerl Change-Id: Ia1789490160f93ef986d8eabb564f3aa78d94dc0 Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/4052 Tested-by: Tomaž Vajngerl Reviewed-by: Miklos Vajna (cherry picked from commit 40306021956262ebeac85f575780904b644f56ec) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208437 Tested-by: Xisco Fauli Reviewed-by: Xisco Fauli --- sc/qa/unit/SwarmSolverTest.cxx | 44 ++++++++++++++++++++++++++++++++ sc/source/solver/SwarmSolver.cxx | 37 ++++++++++++++++++++++----- 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/sc/qa/unit/SwarmSolverTest.cxx b/sc/qa/unit/SwarmSolverTest.cxx index f88e24603187..93dedc64099d 100644 --- a/sc/qa/unit/SwarmSolverTest.cxx +++ b/sc/qa/unit/SwarmSolverTest.cxx @@ -73,6 +73,7 @@ class SwarmSolverTest : public UnoApiTest void testParticleSwarmResultLength(); void testParticleSwarmVelocityNotInitializedAsPosition(); void testUnreadableConstraintStillChecksOthers(); + void testContradictoryBoundsTerminate(); public: SwarmSolverTest() @@ -93,6 +94,7 @@ public: CPPUNIT_TEST(testParticleSwarmResultLength); CPPUNIT_TEST(testParticleSwarmVelocityNotInitializedAsPosition); CPPUNIT_TEST(testUnreadableConstraintStillChecksOthers); + CPPUNIT_TEST(testContradictoryBoundsTerminate); CPPUNIT_TEST_SUITE_END(); }; @@ -651,6 +653,48 @@ void SwarmSolverTest::testUnreadableConstraintStillChecksOthers() !xSolver->getSuccess()); } +void SwarmSolverTest::testContradictoryBoundsTerminate() +{ + // Regression: a variable bounded below 5 and above 10 has an empty, reversed + // range. The reflection in boundVariable used to keep wrapping such a value + // without ever landing inside the range, so solve never returned. It must now + // finish and report the model as not solved. + + loadFromFile(u"Simple.ods"); + + uno::Reference xDocument(mxComponent, uno::UNO_QUERY_THROW); + + uno::Reference xSolver; + xSolver.set(m_xContext->getServiceManager()->createInstanceWithContext( + u"com.sun.star.comp.Calc.SwarmSolver"_ustr, m_xContext), + uno::UNO_QUERY_THROW); + + table::CellAddress aObjective(0, 1, 1); + uno::Sequence aVariables{ { 0, 1, 0 } }; + + // Both act on the variable, so they fold into its bounds: lower 10, upper 5 + uno::Sequence aConstraints{ + { /* [0] Left */ table::CellAddress(0, 1, 0), + /* Operator */ sheet::SolverConstraintOperator_GREATER_EQUAL, + /* Right */ uno::Any(10.0) }, + { /* [1] Left */ table::CellAddress(0, 1, 0), + /* Operator */ sheet::SolverConstraintOperator_LESS_EQUAL, + /* Right */ uno::Any(5.0) } + }; + + xSolver->setDocument(xDocument); + xSolver->setObjective(aObjective); + xSolver->setVariables(aVariables); + xSolver->setConstraints(aConstraints); + xSolver->setMaximize(false); + + // The guarantee here is that solve returns at all. + xSolver->solve(); + + CPPUNIT_ASSERT_MESSAGE("A model with contradictory bounds is not solvable", + !xSolver->getSuccess()); +} + CPPUNIT_TEST_SUITE_REGISTRATION(SwarmSolverTest); } diff --git a/sc/source/solver/SwarmSolver.cxx b/sc/source/solver/SwarmSolver.cxx index b39952c277a0..1d7105fbd97c 100644 --- a/sc/source/solver/SwarmSolver.cxx +++ b/sc/source/solver/SwarmSolver.cxx @@ -365,7 +365,14 @@ void SwarmSolver::initializeVariables(std::vector& rVariables, std::mt19 for (size_t i = 0; i < noVariables; ++i) { Bound const& rBound = maBounds[i]; - if (mbInteger) + if (rBound.lower >= rBound.upper) + { + // An empty or reversed range has nothing to draw from. The + // random distributions are undefined when the lower bound is + // not below the upper one, so take the lower bound directly. + rVariables[i] = rBound.lower; + } + else if (mbInteger) { sal_Int64 intLower(rBound.lower); sal_Int64 intUpper(rBound.upper); @@ -389,6 +396,12 @@ void SwarmSolver::initializeVariables(std::vector& rVariables, std::mt19 double SwarmSolver::clampVariable(size_t nVarIndex, double fValue) { Bound const& rBound = maBounds[nVarIndex]; + + // An empty or reversed range has no room to clamp into. std::clamp with a + // lower bound above the upper bound is undefined, so return the lower bound. + if (rBound.lower >= rBound.upper) + return mbInteger ? std::trunc(rBound.lower) : rBound.lower; + double fResult = std::clamp(fValue, rBound.lower, rBound.upper); if (mbInteger) @@ -400,14 +413,24 @@ double SwarmSolver::clampVariable(size_t nVarIndex, double fValue) double SwarmSolver::boundVariable(size_t nVarIndex, double fValue) { Bound const& rBound = maBounds[nVarIndex]; - // double fResult = std::max(std::min(fValue, rBound.upper), rBound.lower); + double fResult = fValue; - while (fResult < rBound.lower || fResult > rBound.upper) + double const fRange = rBound.upper - rBound.lower; + if (fRange <= 0.0) { - if (fResult < rBound.lower) - fResult = rBound.upper - (rBound.lower - fResult); - if (fResult > rBound.upper) - fResult = (fResult - rBound.upper) + rBound.lower; + // An empty range, where the lower and upper bound are equal, or a + // reversed range from contradictory constraints, holds a single + // value. Return the lower bound. + fResult = rBound.lower; + } + else if (fResult < rBound.lower || fResult > rBound.upper) + { + // Wrap an out of range value back into the range, keeping its offset + // from the lower bound. + double fOffset = std::fmod(fResult - rBound.lower, fRange); + if (fOffset < 0.0) + fOffset += fRange; + fResult = rBound.lower + fOffset; } if (mbInteger)