ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

SwarmSolver: handle an empty or reversed variable range

Browse Source 
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 <tomaz.vajngerl@collabora.co.uk>
Change-Id: Ia1789490160f93ef986d8eabb564f3aa78d94dc0
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/4052
Tested-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
(cherry picked from commit 40306021956262ebeac85f575780904b644f56ec)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208437
Tested-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
This commit is contained in:
Tomaž Vajngerl
2026-07-22 21:51:42 +02:00
committed by Xisco Fauli
parent 5a487764c0
commit 22f84b1b58
2 changed files with 74 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sc/qa/unit/SwarmSolverTest.cxx
+44
View File
@@ -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<sheet::XSpreadsheetDocument> xDocument(mxComponent, uno::UNO_QUERY_THROW);
uno::Reference<sheet::XSolver> 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<table::CellAddress> aVariables{ { 0, 1, 0 } };
// Both act on the variable, so they fold into its bounds: lower 10, upper 5
uno::Sequence<sheet::SolverConstraint> 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);
}
sc/source/solver/SwarmSolver.cxx
+30 -7
View File
@@ -365,7 +365,14 @@ void SwarmSolver::initializeVariables(std::vector<double>& 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<double>& 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)