Fix #1252: symmetric split point in dichotomic ObjectiveStrategy

CHANGES.md

@@ -14,6 +14,12 @@ NEXT MILESTONE
 - XCSP3: handle reified constraints, i.e. `reifiedBy`, `hreifiedFrom` and `hreifiedTo`, which were ignored (#1248). The reification is refused with an explicit message when it cannot be handled, as for `slide` and logic meta-constraints
 - XCSP3: add `XCSPParserTest`, which checks the solutions of small instances against the ones found by brute force
 
+#### Search & Strategies
+- `ObjectiveStrategy`: fix the split point of the `DICHOTOMIC` policy, which was rounded down for both `MINIMIZE` and `MAXIMIZE` (#1252). It is now rounded towards the bound to improve, so that minimizing an objective and maximizing its opposite try mirrored cuts. This changes the cut tried when maximizing
+- `ObjectiveStrategy`: add a `largerHalfFirst` parameter to try the larger half of the objective domain first, also available with `Search.objectiveStrategy(objective, largerHalfFirst)`
+- `ObjectiveStrategy`: a decision that does not reduce the domain of the objective is not returned anymore
+- **Breaking**: `ObjectiveStrategy` has a single constructor, `ObjectiveStrategy(objective, policy, fraction, largerHalfFirst)`. The `int[] coefs` parameter is replaced by a `double` fraction in [0, 1]: `[a, b]` becomes `b / (a + b)`
+
 ### Deprecated API (to be removed in next release):
 
 ### Other closed issues and pull requests:

solver/src/main/java/org/chocosolver/solver/objective/ObjectiveStrategy.java

@@ -38,14 +38,14 @@ public class ObjectiveStrategy extends AbstractStrategy<IntVar> {
     //***********************************************************************************
 
     private int globalLB, globalUB;
-    private final int coefLB;
-    private final int coefUB;
     private final IntVar obj;
     private long nbSols;
     private final Model model;
     private boolean firstCall;
     private DecisionOperator<IntVar> decOperator;
     private final OptimizationPolicy optPolicy;
+    private final double fraction;
+    private final boolean largerHalfFirst;
     public final DecisionOperator<IntVar> decUB = new BottomUpDecisionOperator();
     public final DecisionOperator<IntVar> incLB = new TopDownDecisionOperator();
     //***********************************************************************************
@@ -55,53 +55,40 @@ public class ObjectiveStrategy extends AbstractStrategy<IntVar> {
     /**
      * Defines a branching strategy over the objective variable
      * BEWARE: only activated after a first solution
+     * <p>
+     * With DICHOTOMIC, the domain [LB, UB] of the objective is split at
+     * {@code LB + fraction * (UB - LB)}: 0.5 halves the domain, a greater value takes a split
+     * point closer to the upper bound, a lower value one closer to the lower bound.
+     * When the split point is not an integer, it is rounded so that minimizing an objective
+     * with {@code fraction} and maximizing its opposite with {@code 1 - fraction} try mirrored
+     * cuts. The direction of the rounding is given by {@code largerHalfFirst}:
+     * <ul>
+     * <li>{@code false}: the branch tried first keeps the smaller part of the domain
+     * (rounded down when minimizing, up when maximizing), the cut is more aggressive;</li>
+     * <li>{@code true}: the branch tried first keeps the larger part of the domain
+     * (rounded up when minimizing, down when maximizing), the cut is more cautious
+     * but its refutation is stronger.</li>
+     * </ul>
      *
-     * @param objective variable
-     * @param policy    BOTTOM_UP, TOP_TOWN or DICHOTOMIC
+     * @param objective       variable
+     * @param policy          BOTTOM_UP, TOP_TOWN or DICHOTOMIC
+     * @param fraction        with DICHOTOMIC, position of the split point in the domain,
+     *                        in [0, 1]; ignored otherwise
+     * @param largerHalfFirst with DICHOTOMIC, set to <i>true</i> to try first the larger part
+     *                        of the domain, <i>false</i> to try first the smaller one
      */
-    public ObjectiveStrategy(IntVar objective, OptimizationPolicy policy) {
-        this(objective, getCoefs(policy), policy);
-    }
-
-    /**
-     * Defines a parametrized dichotomic branching over the objective variable
-     * BEWARE: only activated after a first solution
-     *
-     * @param objective variable
-     * @param coefs     [a,b] defines how to split the domain of the objective variable
-     *                  [1,1] will halve its domain
-     *                  [1,2] will take a value closer to the upper bound than the lower bound
-     * @param policy    should be DICHOTOMIC
-     */
-    public ObjectiveStrategy(IntVar objective, int[] coefs, OptimizationPolicy policy) {
-        super(new IntVar[]{objective});
+    public ObjectiveStrategy(IntVar objective, OptimizationPolicy policy, double fraction, boolean largerHalfFirst) {
+        super(objective);
+        if (Double.isNaN(fraction) || fraction < 0. || fraction > 1.) {
+            throw new IllegalArgumentException("fraction should be in [0, 1], not " + fraction);
+        }
+        this.fraction = fraction;
+        this.largerHalfFirst = largerHalfFirst;
         this.obj = objective;
         this.model = obj.getModel();
         this.firstCall = true;
-        this.coefLB = coefs[0];
-        this.coefUB = coefs[1];
         this.optPolicy = policy;
         model.getSolver().setRestartOnSolutions();
-        if (coefLB < 0 || coefUB < 0 || coefLB + coefUB == 0) {
-            throw new UnsupportedOperationException("coefLB<0, coefUB<0 and coefLB+coefUB==0 are forbidden");
-        }
-        if (coefLB + coefUB != 1 && policy != DICHOTOMIC) {
-            throw new UnsupportedOperationException("Invalid coefficients for BOTTOM_UP or TOP_DOWN optimization" +
-                    "\nuse signature public ObjectiveStrategy(IntVar obj, OptimizationPolicy policy, Model model) instead");
-        }
-    }
-
-    private static int[] getCoefs(OptimizationPolicy policy) {
-        switch (policy) {
-            case BOTTOM_UP:
-                return new int[]{1, 0};
-            case TOP_DOWN:
-                return new int[]{0, 1};
-            case DICHOTOMIC:
-                return new int[]{1, 1};
-            default:
-                throw new UnsupportedOperationException("unknown OptimizationPolicy " + policy);
-        }
     }
 
     private DecisionOperator<IntVar> getOperator(OptimizationPolicy optPolicy, ResolutionPolicy resoPolicy) {
@@ -165,15 +152,32 @@ public Decision<IntVar> getDecision() {
         if (model.getSettings().warnUser()) {
             model.getSolver().log().bold().println("- objective in [" + globalLB + ", " + globalUB + "]");
         }
-        int target;
-        target = (globalLB * coefLB + globalUB * coefUB) / (coefLB + coefUB);
+        int target = getTarget();
+        if (decOperator == incLB ? target <= obj.getLB() : target >= obj.getUB()) {
+            // the decision would not reduce the domain
+            return null;
+        }
         IntDecision dec = decisionPath.makeIntDecision(obj, decOperator, target);
         if (model.getSettings().warnUser()) {
             model.getSolver().log().bold().println("- trying " + obj + " " + (decOperator == decUB ? "<=" : ">=") + " " + target);
         }
         return dec;
     }
 
+    private int getTarget() {
+        switch (optPolicy) {
+            case BOTTOM_UP:
+                return globalLB;
+            case TOP_DOWN:
+                return globalUB;
+            default:
+                double split = globalLB + fraction * ((double) globalUB - globalLB);
+                // rounded towards the bound to improve, unless the larger half is tried first
+                boolean roundUp = (decOperator == incLB) != largerHalfFirst;
+                return (int) (roundUp ? Math.ceil(split) : Math.floor(split));
+        }
+    }
+
     public class BottomUpDecisionOperator implements DecisionOperator<IntVar> {
         //FIXME can not serialize decision 
 
@@ -191,7 +195,7 @@ public boolean unapply(IntVar var, int value, ICause cause) throws Contradiction
 
         @Override
         public String toString() {
-            return " objective split(" + coefLB + "," + coefUB + "), decreases the upper bound first";
+            return " objective " + optPolicy + ", decreases the upper bound first";
         }
 
         @Override
@@ -216,7 +220,7 @@ public boolean unapply(IntVar var, int value, ICause cause) throws Contradiction
 
         @Override
         public String toString() {
-            return " objective split(" + coefLB + "," + coefUB + "), increases the lower bound first";
+            return " objective " + optPolicy + ", increases the lower bound first";
         }
 
         @Override

solver/src/main/java/org/chocosolver/solver/search/strategy/Search.java

@@ -613,7 +613,24 @@ public static IntStrategy randomSearch(IntVar[] vars, long seed) {
      */
     public static AbstractStrategy<IntVar> objectiveStrategy(IntVar objective,
                                                              OptimizationPolicy optPolicy) {
-        return new ObjectiveStrategy(objective, optPolicy);
+        return new ObjectiveStrategy(objective, optPolicy, 0.5, false);
+    }
+
+    /**
+     * Defines a dichotomic branching strategy over the objective variable Note that it is only
+     * activated after a first solution. This should be completed with another strategy with a
+     * larger scope.
+     *
+     * @param objective       objective variable
+     * @param largerHalfFirst set to <i>true</i> to try first the larger half of the objective
+     *                        domain (cautious cut, strong refutation), <i>false</i> to try first
+     *                        the smaller one (aggressive cut, weak refutation)
+     * @return a assignment strategy
+     * @see ObjectiveStrategy#ObjectiveStrategy(IntVar, OptimizationPolicy, double, boolean)
+     */
+    public static AbstractStrategy<IntVar> objectiveStrategy(IntVar objective,
+                                                             boolean largerHalfFirst) {
+        return new ObjectiveStrategy(objective, OptimizationPolicy.DICHOTOMIC, 0.5, largerHalfFirst);
     }
 
     /**

solver/src/test/java/org/chocosolver/solver/constraints/graph/cost/trees/DCMST.java

@@ -8,9 +8,9 @@
 
 import org.chocosolver.solver.Model;
 import org.chocosolver.solver.Solver;
-import org.chocosolver.solver.objective.ObjectiveStrategy;
 import org.chocosolver.solver.objective.OptimizationPolicy;
 import org.chocosolver.solver.search.loop.monitors.IMonitorSolution;
+import org.chocosolver.solver.search.strategy.Search;
 import org.chocosolver.solver.search.strategy.strategy.GraphCostBasedSearch;
 import org.chocosolver.solver.variables.IntVar;
 import org.chocosolver.solver.variables.UndirectedGraphVar;
@@ -87,7 +87,7 @@ public void solve() {
             System.out.println("Solution found : "+totalCost);
         });
         // bottom-up optimization : find a first solution then reach the global minimum from below
-        s.setSearch(new ObjectiveStrategy(totalCost, OptimizationPolicy.BOTTOM_UP), mainSearch);
+        s.setSearch(Search.objectiveStrategy(totalCost, OptimizationPolicy.BOTTOM_UP), mainSearch);
         s.limitSolution(2); // therefore there is at most two solutions
         long TIMELIMIT = 300000;
         s.limitTime(TIMELIMIT); // time limit

solver/src/test/java/org/chocosolver/solver/search/ObjectiveTest.java

@@ -16,6 +16,7 @@
 import org.chocosolver.solver.constraints.Constraint;
 import org.chocosolver.solver.objective.IObjectiveManager;
 import org.chocosolver.solver.objective.ObjectiveFactory;
+import org.chocosolver.solver.objective.ObjectiveStrategy;
 import org.chocosolver.solver.objective.OptimizationPolicy;
 import org.chocosolver.solver.search.loop.monitors.IMonitorSolution;
 import org.chocosolver.solver.search.strategy.Search;
@@ -30,6 +31,8 @@
 import org.chocosolver.solver.variables.Variable;
 import org.testng.annotations.Test;
 
+import gnu.trove.list.array.TIntArrayList;
+
 import java.io.IOException;
 import java.util.Random;
 
@@ -484,4 +487,51 @@ public void testCP2() {
             assertFalse(model.getSolver().isStopCriterionMet());
         }
     }
+
+    /**
+     * Solves a problem reduced to its objective variable, whose first solution is the worst one,
+     * and returns the successive values of the objective.
+     */
+    private static int[] dichotomicTrace(boolean maximize, int lb, int ub, boolean largerHalfFirst) {
+        return dichotomicTrace(maximize, lb, ub, 0.5, largerHalfFirst);
+    }
+
+    private static int[] dichotomicTrace(boolean maximize, int lb, int ub, double fraction, boolean largerHalfFirst) {
+        Model model = new Model();
+        IntVar obj = model.intVar("obj", lb, ub, false);
+        model.setObjective(maximize, obj);
+        Solver solver = model.getSolver();
+        solver.setSearch(new ObjectiveStrategy(obj, OptimizationPolicy.DICHOTOMIC, fraction, largerHalfFirst),
+                maximize ? inputOrderLBSearch(obj) : inputOrderUBSearch(obj));
+        TIntArrayList values = new TIntArrayList();
+        while (solver.solve()) {
+            values.add(obj.getValue());
+        }
+        assertFalse(solver.isStopCriterionMet());
+        return values.toArray();
+    }
+
+    @Test(groups = "1s", timeOut = 60000)
+    public void testDichotomicIsSymmetric() {
+        // minimizing f in [7, 11] and maximizing 35 - f or -f must try mirrored cuts
+        assertEquals(dichotomicTrace(false, 7, 11, false), new int[]{11, 8, 7});
+        assertEquals(dichotomicTrace(true, 24, 28, false), new int[]{24, 27, 28});
+        assertEquals(dichotomicTrace(true, -11, -7, false), new int[]{-11, -8, -7});
+        assertEquals(dichotomicTrace(false, -28, -24, false), new int[]{-24, -27, -28});
+
+        assertEquals(dichotomicTrace(false, 7, 11, true), new int[]{11, 9, 8, 7});
+        assertEquals(dichotomicTrace(true, 24, 28, true), new int[]{24, 26, 27, 28});
+        assertEquals(dichotomicTrace(true, -11, -7, true), new int[]{-11, -9, -8, -7});
+        assertEquals(dichotomicTrace(false, -28, -24, true), new int[]{-24, -26, -27, -28});
+    }
+
+    @Test(groups = "1s", timeOut = 60000)
+    public void testDichotomicFraction() {
+        // minimizing f with fraction 0.75 and maximizing 35 - f with fraction 0.25 must try mirrored cuts
+        assertEquals(dichotomicTrace(false, 7, 11, 0.75, false), new int[]{11, 9, 7});
+        assertEquals(dichotomicTrace(true, 24, 28, 0.25, false), new int[]{24, 26, 28});
+        assertEquals(dichotomicTrace(false, 7, 11, 0.75, true), new int[]{11, 10, 9, 8, 7});
+        assertEquals(dichotomicTrace(true, 24, 28, 0.25, true), new int[]{24, 25, 26, 27, 28});
+        assertThrows(IllegalArgumentException.class, () -> dichotomicTrace(false, 7, 11, 1.5, false));
+    }
 }