SwarmSolver: skip unreadable constraint instead of passing the point

When a constraint's right hand side could not be read as a cell or a
number, the feasibility check returned "satisfied" for the whole
point. That stopped the loop and hid every constraint after the
unreadable one, so an infeasible model could be reported as solved.
Skip the constraint that cannot be evaluated and carry on checking
the rest.

Add a test that places an unreadable constraint before an impossible
one and checks the model is still reported as not solved.

Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: I51fb10e3a7c1863ad24e1899fa51d4aaf6382349
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/4051
Tested-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
(cherry picked from commit 216843854e4035fe71a39e908b744d67863dd406)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208436
Tested-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>

sc/qa/unit/SwarmSolverTest.cxx

@@ -72,6 +72,7 @@ class SwarmSolverTest : public UnoApiTest
     void testLargeObjectiveStillSolvable_Particle_Swarm();
     void testParticleSwarmResultLength();
     void testParticleSwarmVelocityNotInitializedAsPosition();
+    void testUnreadableConstraintStillChecksOthers();
 
 public:
     SwarmSolverTest()
@@ -91,6 +92,7 @@ public:
     CPPUNIT_TEST(testLargeObjectiveStillSolvable_Particle_Swarm);
     CPPUNIT_TEST(testParticleSwarmResultLength);
     CPPUNIT_TEST(testParticleSwarmVelocityNotInitializedAsPosition);
+    CPPUNIT_TEST(testUnreadableConstraintStillChecksOthers);
     CPPUNIT_TEST_SUITE_END();
 };
 
@@ -604,6 +606,51 @@ void SwarmSolverTest::testParticleSwarmVelocityNotInitializedAsPosition()
     CPPUNIT_ASSERT_EQUAL(8, aProvider.mnInitCalls);
 }
 
+void SwarmSolverTest::testUnreadableConstraintStillChecksOthers()
+{
+    // Regression: a constraint whose right hand side could not be read as a cell
+    // or a number used to make the feasibility check return "satisfied" for the
+    // whole point, hiding every later constraint. Here an unreadable constraint
+    // comes before an impossible one, so the model must still be reported 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 constraints act on the objective cell, not the variable, so they end
+    // up as non-bounded constraints in the order given here.
+    uno::Sequence<sheet::SolverConstraint> aConstraints{
+        // right hand side is a string, so it reads as neither a cell nor a
+        // number
+        { /* [0] Left     */ table::CellAddress(0, 1, 1),
+          /*     Operator */ sheet::SolverConstraintOperator_LESS_EQUAL,
+          /*     Right    */ uno::Any(u"not a number"_ustr) },
+        // forced below the objective's global minimum of -130, so impossible
+        { /* [1] Left     */ table::CellAddress(0, 1, 1),
+          /*     Operator */ sheet::SolverConstraintOperator_LESS_EQUAL,
+          /*     Right    */ uno::Any(-1000.0) }
+    };
+
+    xSolver->setDocument(xDocument);
+    xSolver->setObjective(aObjective);
+    xSolver->setVariables(aVariables);
+    xSolver->setConstraints(aConstraints);
+    xSolver->setMaximize(false);
+
+    xSolver->solve();
+
+    CPPUNIT_ASSERT_MESSAGE("an unreadable constraint must not hide a later one",
+                           !xSolver->getSuccess());
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SwarmSolverTest);
 }
 

sc/source/solver/SwarmSolver.cxx

@@ -437,7 +437,10 @@ bool SwarmSolver::doesViolateConstraints()
         }
         else
         {
-            return false;
+            // The right hand side is neither a cell nor a number, so this
+            // constraint cannot be evaluated. Skip it and carry on checking the
+            // rest of the constraints.
+            continue;
         }
 
         sheet::SolverConstraintOperator eOp = rConstraint.Operator;