Fix dual accessors for original knapsack constraints and the getVarsAnd check

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Added type annotations to most methods on the `Model` class
 - Added tests for `getRowLinear()` and extended existing testing for `isActive()`
 ### Fixed
+- Fixed `getDualsolKnapsack()` and `getDualfarkasKnapsack()` returning 0 for original constraints, and the handler check in `getVarsAnd()` that could never fail
 - Fixed Cython 3.3 compatibility (#1248)
 - Made `test_markDoNotAggrVar_and_getStatus` robust to SCIP presolve changes by discovering the aggregated/multi-aggregated variables instead of hardcoding them
 ### Changed

src/pyscipopt/scip.pxi

@@ -6953,7 +6953,7 @@ cdef class Model:
         cdef int i
 
         constype = bytes(SCIPconshdlrGetName(SCIPconsGetHdlr(and_cons.scip_cons))).decode('UTF-8')
-        assert(constype == 'and', "The constraint handler %s does not have this functionality." % constype)
+        assert constype == 'and', "The constraint handler %s does not have this functionality." % constype
 
         nvars = SCIPgetNVarsAnd(self._scip, and_cons.scip_cons)
         _vars = SCIPgetVarsAnd(self._scip, and_cons.scip_cons)
@@ -8960,14 +8960,19 @@ cdef class Model:
         float
 
         """
+        cdef SCIP_CONS* transcons
+
         constype = bytes(SCIPconshdlrGetName(SCIPconsGetHdlr(cons.scip_cons))).decode('UTF-8')
         if not constype == 'knapsack':
             raise Warning("dual solution values not available for constraints of type ", constype)
         if cons.isOriginal():
-            transcons = cons
+            # presolve may have removed it, and then there's no dual to read
+            PY_SCIP_CALL(SCIPgetTransformedCons(self._scip, cons.scip_cons, &transcons))
+            if transcons == NULL:
+                return 0.0
         else:
-            transcons = <Constraint>self.getTransformedCons(cons)
-        return SCIPgetDualsolKnapsack(self._scip, transcons.scip_cons)
+            transcons = cons.scip_cons
+        return SCIPgetDualsolKnapsack(self._scip, transcons)
 
     def getDualMultiplier(self, Constraint cons):
         """
@@ -9020,12 +9025,17 @@ cdef class Model:
         float
 
         """
+        cdef SCIP_CONS* transcons
+
         # TODO this should ideally be handled on the SCIP side
         if cons.isOriginal():
-            return SCIPgetDualfarkasKnapsack(self._scip, cons.scip_cons)
+            # presolve may have removed it, and then there's no Farkas value to read
+            PY_SCIP_CALL(SCIPgetTransformedCons(self._scip, cons.scip_cons, &transcons))
+            if transcons == NULL:
+                return 0.0
+            return SCIPgetDualfarkasKnapsack(self._scip, transcons)
         else:
-            transcons = <Constraint>self.getTransformedCons(cons)
-            return SCIPgetDualfarkasKnapsack(self._scip, transcons.scip_cons)
+            return SCIPgetDualfarkasKnapsack(self._scip, cons.scip_cons)
 
     def getVarRedcost(self, Variable var):
         """

tests/test_cons.py

@@ -352,6 +352,57 @@ def test_cons_knapsack():
     assert m.getDualsolKnapsack(knapsack_cons) == 0
     assert m.getDualfarkasKnapsack(knapsack_cons) == 0
 
+
+def _lp_only_model():
+    # presolve, heuristics, separation and propagation off, so the constraint
+    # stays in the LP and its dual values are still there after the solve
+    m = Model()
+    m.hideOutput()
+    m.setPresolve(SCIP_PARAMSETTING.OFF)
+    m.setHeuristics(SCIP_PARAMSETTING.OFF)
+    m.setSeparating(SCIP_PARAMSETTING.OFF)
+    m.disablePropagation()
+    return m
+
+
+def test_cons_knapsack_dualsol_on_original_constraint():
+    m = _lp_only_model()
+    x = m.addVar(vtype="B")
+    y = m.addVar(vtype="B")
+    knapsack_cons = m.addConsKnapsack([x, y], [1, 1], 1)
+    m.setObjective(-x - y, "minimize")
+    m.optimize()
+
+    # the knapsack is binding and its dual is unique here, so asking through
+    # the original constraint has to give the same value as the transformed one
+    transformed = m.getTransformedCons(knapsack_cons)
+    assert m.isEQ(m.getDualsolKnapsack(knapsack_cons), -1.0)
+    assert m.isEQ(m.getDualsolKnapsack(knapsack_cons), m.getDualsolKnapsack(transformed))
+
+
+def test_cons_knapsack_dualfarkas_on_original_constraint():
+    m = _lp_only_model()
+    x = m.addVar(vtype="B")
+    y = m.addVar(vtype="B")
+    knapsack_cons = m.addConsKnapsack([x, y], [1, 1], 1)
+    m.addCons(x + y >= 2)
+    m.optimize()
+
+    assert m.getStatus() == "infeasible"
+    transformed = m.getTransformedCons(knapsack_cons)
+    assert not m.isZero(m.getDualfarkasKnapsack(knapsack_cons))
+    assert m.isEQ(m.getDualfarkasKnapsack(knapsack_cons), m.getDualfarkasKnapsack(transformed))
+
+
+def test_getVarsAnd_rejects_other_constraint_types():
+    m = Model()
+    x = m.addVar(vtype="B")
+    linear_cons = m.addCons(x <= 1)
+
+    with pytest.raises(AssertionError):
+        m.getVarsAnd(linear_cons)
+
+
 def test_cons_cumulative():
     """Three jobs on a resource with capacity 3 must not overlap in demand.