Remove requirement to freeze and unfreeze for delta monitors.

Consequences: - all values are consumed on a call to `forEach...`) - fixes a bug related to inc. propagators, views and missed events (see `TableTest` for an example)
chocoteam · Dec 17, 2020 · 84a3fc3 · 84a3fc3
1 parent 45227c4
commit 84a3fc3
Show file tree

Hide file tree

Showing 39 changed files with 756 additions and 910 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -8,6 +8,10 @@ NEXT MILESTONE
 -------------------
 
 ### Major features:
+- Simplify the way deltamonitors work. There is no need to `freeze` and `unfreeze` 
+them before calling `forEach...` methods. But, a call to `forEach...` consumes all values stored.
+- Fix a bug related to incremental propagators, views and missing events.
+
 
 ### Deprecated API (to be removed in next release):
 
@@ -18,7 +22,7 @@ See [milestone 4.10.7](https://github.com/chocoteam/choco-solver/milestone/xx)
 
 
 4.10.6 - 11 Dec 2020
--------------------             A
+-------------------             
 
 ### Major features:
 - Add new resolution helper in `Solver`, namely `findOptimalSolutionWithBounds`. See Javadoc for details and usages.

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropDistanceXYC.java b/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropDistanceXYC.java
@@ -93,9 +93,6 @@ public void propagate(int evtmask) throws ContradictionException {
         } else {
             filterNeq();
         }
-        for (int i = 0; i < idms.length; i++) {
-            idms[i].unfreeze();
-        }
     }
 
     @Override
@@ -107,9 +104,7 @@ public void propagate(int varIdx, int mask) throws ContradictionException {
                     filterOnInst(vars[idx2], vars[varIdx].getValue());
                 } else {
                     if (IntEventType.isRemove(mask) && vars[varIdx].hasEnumeratedDomain()) {
-                        idms[varIdx].freeze();
                         idms[varIdx].forEachRemVal(remproc.set(varIdx));
-                        idms[varIdx].unfreeze();
                     }
                     if (IntEventType.isInclow(mask)) {
                         filterOnInf(vars[varIdx], vars[idx2]);

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropEqualXY_C.java b/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropEqualXY_C.java
@@ -79,8 +79,6 @@ public void propagate(int evtmask) throws ContradictionException {
                     y.removeValue(val, this);
                 }
             }
-            idms[0].unfreeze();
-            idms[1].unfreeze();
         }
         if (x.isInstantiated()) {
             assert (y.isInstantiated());
@@ -96,9 +94,7 @@ public void propagate(int varIdx, int mask) throws ContradictionException {
             setPassive();
         } else if (bothEnumerated) {
             indexToFilter = 1 - varIdx;
-            idms[varIdx].freeze();
             idms[varIdx].forEachRemVal(rem_proc);
-            idms[varIdx].unfreeze();
         }
     }
 

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropEqualX_Y.java b/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropEqualX_Y.java
@@ -82,8 +82,6 @@ public void propagate(int evtmask) throws ContradictionException {
                     y.removeValue(val, this);
                 }
             }
-            idms[0].unfreeze();
-            idms[1].unfreeze();
         }
         if (x.isInstantiated()) {
             assert (y.isInstantiated());
@@ -102,9 +100,7 @@ public void propagate(int varIdx, int mask) throws ContradictionException {
             setPassive();
         } else if (bothEnumerated) {
             indexToFilter = 1 - varIdx;
-            idms[varIdx].freeze();
             idms[varIdx].forEachRemVal(rem_proc);
-            idms[varIdx].unfreeze();
         }
     }
 

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropEqualX_YC.java b/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropEqualX_YC.java
@@ -81,8 +81,6 @@ public void propagate(int evtmask) throws ContradictionException {
                     y.removeValue(val, this);
                 }
             }
-            idms[0].unfreeze();
-            idms[1].unfreeze();
         }
         if (x.isInstantiated()) {
             assert (y.isInstantiated());
@@ -104,9 +102,7 @@ public void propagate(int varIdx, int mask) throws ContradictionException {
                 indexToFilter = 0;
                 offSet = cste;
             }
-            idms[varIdx].freeze();
             idms[varIdx].forEachRemVal(rem_proc);
-            idms[varIdx].unfreeze();
         }
     }
 

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropSquare.java b/solver/src/main/java/org/chocosolver/solver/constraints/binary/PropSquare.java
@@ -54,9 +54,6 @@ public void propagate(int evtmask) throws ContradictionException {
         updateLowerBoundofY();
         updateUpperBoundofY();
         updateHolesinY();
-        for (int i = 0; i < idms.length; i++) {
-            idms[i].unfreeze();
-        }
     }
 
     @Override
@@ -67,9 +64,7 @@ public void propagate(int varIdx, int mask) throws ContradictionException {
                 updateUpperBoundofY();
                 updateHolesinY();
             } else {
-                idms[varIdx].freeze();
                 idms[varIdx].forEachRemVal(rem_proc.set(varIdx));
-                idms[varIdx].unfreeze();
 //                updateHolesinY();
             }
         } else { // filter from Y to X
@@ -82,9 +77,7 @@ public void propagate(int varIdx, int mask) throws ContradictionException {
                 updateUpperBoundofX();
                 updateHolesinX();
             } else {
-                idms[varIdx].freeze();
                 idms[varIdx].forEachRemVal(rem_proc.set(varIdx));
-                idms[varIdx].unfreeze();
 //                updateHolesinX();
             }
         }

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/extension/nary/PropCompactTable.java b/solver/src/main/java/org/chocosolver/solver/constraints/extension/nary/PropCompactTable.java
@@ -139,15 +139,11 @@ public void propagate(int evtmask) throws ContradictionException {
             }
         }
         filterDomains();
-        for (int i = 0; i < vars.length; i++) {
-            monitors[i].unfreeze();
-        }
     }
 
     @Override
     public void propagate(int vIdx, int mask) throws ContradictionException {
         currTable.clearMask();
-        monitors[vIdx].freeze();
         if (vars[vIdx].getDomainSize() > monitors[vIdx].sizeApproximation()) {
             monitors[vIdx].forEachRemVal(onValRem.set(vIdx));
             currTable.reverseMask();
@@ -158,7 +154,6 @@ public void propagate(int vIdx, int mask) throws ContradictionException {
             }
         }
         currTable.intersectWithMask();
-        monitors[vIdx].unfreeze();
         if (currTable.isEmpty()) { // fail as soon as possible
             fails();
         }

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/nary/automata/PropCostRegular.java b/solver/src/main/java/org/chocosolver/solver/constraints/nary/automata/PropCostRegular.java
@@ -90,16 +90,11 @@ public void propagate(int evtmask) throws ContradictionException {
         if (PropagatorEventType.isFullPropagation(evtmask)) {
             // as the graph was build on initial domain, this is allowed (specific case)
             for (int i = 0; i < idms.length; i++) {
-                idms[i].freeze();
                 idms[i].forEachRemVal(rem_proc.set(i));
             }
             initialize();
         }
         filter();
-		// added by JG: the propagator should be idempotent so it should not iterate over its own removals
-		for (int i = 0; i < idms.length; i++) {
-			idms[i].unfreeze();
-		}
     }
 
     @Override
@@ -108,9 +103,7 @@ public void propagate(int varIdx, int mask) throws ContradictionException {
         if (varIdx == zIdx) { // z only deals with bound events
             boundChange.set(true);
         } else { // other variables only deals with removal events
-            idms[varIdx].freeze();
             idms[varIdx].forEachRemVal(rem_proc.set(varIdx));
-            idms[varIdx].unfreeze();
         }
         forcePropagate(PropagatorEventType.CUSTOM_PROPAGATION);
     }

diff --git a/.../src/main/java/org/chocosolver/solver/constraints/nary/automata/PropMultiCostRegular.java b/.../src/main/java/org/chocosolver/solver/constraints/nary/automata/PropMultiCostRegular.java
@@ -14,11 +14,6 @@
 import gnu.trove.set.hash.TIntHashSet;
 import gnu.trove.stack.TIntStack;
 import gnu.trove.stack.array.TIntArrayStack;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.BitSet;
-import java.util.HashSet;
-import java.util.List;
 import org.chocosolver.memory.IEnvironment;
 import org.chocosolver.solver.constraints.Propagator;
 import org.chocosolver.solver.constraints.PropagatorPriority;
@@ -42,6 +37,8 @@
 import org.chocosolver.util.tools.ArrayUtils;
 import org.jgrapht.graph.DirectedMultigraph;
 
+import java.util.*;
+
 
 /**
  * Created by IntelliJ IDEA.
@@ -240,19 +237,13 @@ public void propagate(int evtmask) throws ContradictionException {
             initialize();
         }
         filter();
-		// added by JG: the propagator should be idempotent so it should not iterate over its own removals
-		for (int i = 0; i < idms.length; i++) {
-			idms[i].unfreeze();
-		}
     }
 
     @Override
     public void propagate(int varIdx, int mask) throws ContradictionException {
         if (varIdx < offset) {
             checkWorld();
-            idms[varIdx].freeze();
             idms[varIdx].forEachRemVal(rem_proc.set(varIdx));
-            idms[varIdx].unfreeze();
         } else {// if (EventType.isInstantiate(mask) || EventType.isBound(mask)) {
             boundUpdate.add(varIdx - offset);
             computed = false;

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/nary/automata/PropRegular.java b/solver/src/main/java/org/chocosolver/solver/constraints/nary/automata/PropRegular.java
@@ -59,22 +59,18 @@ public PropRegular(IntVar[] variables, IAutomaton automaton) {
     public void propagate(int evtmask) throws ContradictionException {
         assert evtmask == PropagatorEventType.FULL_PROPAGATION.getMask();
         for (int i = 0; i < idms.length; i++) {
-            idms[i].freeze(); // as the graph was build on initial domain, this is allowed (specific case)
             idms[i].forEachRemVal(rem_proc.set(i));
             for (int j = vars[i].getLB(); j <= vars[i].getUB(); j = vars[i].nextValue(j)) {
                 if (!graph.hasSupport(i, j)) {
                     vars[i].removeValue(j, this);
                 }
             }
-            idms[i].unfreeze();
         }
     }
 
     @Override
     public void propagate(int varIdx, int mask) throws ContradictionException {
-        idms[varIdx].freeze();
         idms[varIdx].forEachRemVal(rem_proc.set(varIdx));
-        idms[varIdx].unfreeze();
     }
 
     @Override

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/nary/binPacking/PropBinPacking.java b/solver/src/main/java/org/chocosolver/solver/constraints/nary/binPacking/PropBinPacking.java
@@ -9,9 +9,6 @@
  */
 package org.chocosolver.solver.constraints.nary.binPacking;
 
-import java.util.BitSet;
-import java.util.Comparator;
-import java.util.stream.IntStream;
 import org.chocosolver.memory.IStateInt;
 import org.chocosolver.solver.constraints.Propagator;
 import org.chocosolver.solver.constraints.PropagatorPriority;
@@ -28,6 +25,10 @@
 import org.chocosolver.util.procedure.UnaryIntProcedure;
 import org.chocosolver.util.tools.ArrayUtils;
 
+import java.util.BitSet;
+import java.util.Comparator;
+import java.util.stream.IntStream;
+
 /**
  * Propagator for a Bin Packing constraint
  * This propagator is an implementation of filtering rules introduced in the following paper :
@@ -329,9 +330,7 @@ private boolean noSumFiltering(int j) throws ContradictionException {
     @Override
     public void propagate(int idxVarInProp, int mask) throws ContradictionException {
         if(idxVarInProp < nbItems) {
-            monitors[idxVarInProp].freeze();
             monitors[idxVarInProp].forEachRemVal(procedure.set(idxVarInProp));
-            monitors[idxVarInProp].unfreeze();
             if(itemBin[idxVarInProp].isInstantiated()) {
                 int j = itemBin[idxVarInProp].getValue() - offset;
                 updateRAfterInstantiation(j, idxVarInProp);
@@ -360,9 +359,6 @@ public void propagate(int evtmask) throws ContradictionException {
                 }
             }
             binsToProcess.set(0, nbAvailableBins);
-            for(int i=0; i<nbItems; i++){
-                monitors[i].unfreeze();
-            }
         }
         while(!binsToProcess.isEmpty()) {
             processBin(binsToProcess.nextSetBit(0));

diff --git a/...rc/main/java/org/chocosolver/solver/constraints/nary/channeling/PropClauseChanneling.java b/...rc/main/java/org/chocosolver/solver/constraints/nary/channeling/PropClauseChanneling.java
@@ -117,15 +117,11 @@ public void propagate(int evtmask) throws ContradictionException {
 
         LB.set(lb);
         UB.set(ub);
-
-        // finally delta monitor
-        dm.unfreeze();
     }
 
     @Override
     public void propagate(int vidx, int mask) throws ContradictionException {
         if (vidx == 0) { //iv has been modified
-            dm.freeze();
             if (IntEventType.isInstantiate(mask)) {
                 _inst(iv.getValue() - OFFSET);
             } else {
@@ -145,7 +141,6 @@ public void propagate(int vidx, int mask) throws ContradictionException {
                     }
                 });
             }
-            dm.unfreeze();
         } else {
             vidx--; // idx in eqs or lqs
             int act = 0;

diff --git a/...ain/java/org/chocosolver/solver/constraints/nary/channeling/PropEnumDomainChanneling.java b/...ain/java/org/chocosolver/solver/constraints/nary/channeling/PropEnumDomainChanneling.java
@@ -67,15 +67,12 @@ public void propagate(int evtmask) throws ContradictionException {
                 }
             }
         }
-        idm.unfreeze();
     }
 
     @Override
     public void propagate(int varIdx, int mask) throws ContradictionException {
         if (varIdx == n) {
-            idm.freeze();
             idm.forEachRemVal(rem_proc);
-            idm.unfreeze();
         } else {
             if (vars[varIdx].getValue() == 1) {
                 vars[n].instantiateTo(varIdx + offSet, this);

diff --git a/...rc/main/java/org/chocosolver/solver/constraints/nary/channeling/PropInverseChannelAC.java b/...rc/main/java/org/chocosolver/solver/constraints/nary/channeling/PropInverseChannelAC.java
@@ -68,16 +68,11 @@ public void propagate(int evtmask) throws ContradictionException {
             enumeratedFilteringOfX(i);
             enumeratedFilteringOfY(i);
         }
-        for (int i = 0; i < vars.length; i++) {
-            idms[i].unfreeze();
-        }
     }
 
     @Override
     public void propagate(int varIdx, int mask) throws ContradictionException {
-        idms[varIdx].freeze();
         idms[varIdx].forEachRemVal(rem_proc.set(varIdx));
-        idms[varIdx].unfreeze();
     }
 
     private void enumeratedFilteringOfX(int var) throws ContradictionException {

diff --git a/...r/src/main/java/org/chocosolver/solver/constraints/nary/nvalue/PropAtLeastNValues_AC.java b/...r/src/main/java/org/chocosolver/solver/constraints/nary/nvalue/PropAtLeastNValues_AC.java
@@ -285,17 +285,12 @@ public void propagate(int evtmask) throws ContradictionException {
         if (vars[n].getLB() == card) {
             filter();
         }
-        for (int i = 0; i < idms.length; i++) {
-            idms[i].unfreeze();
-        }
     }
 
     @Override
     public void propagate(int varIdx, int mask) throws ContradictionException {
         if (varIdx < n) {
-            idms[varIdx].freeze();
             idms[varIdx].forEachRemVal(remProc.set(varIdx));
-            idms[varIdx].unfreeze();
         }
         forcePropagate(PropagatorEventType.CUSTOM_PROPAGATION);
     }

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/set/PropAllDisjoint.java b/solver/src/main/java/org/chocosolver/solver/constraints/set/PropAllDisjoint.java
@@ -85,17 +85,12 @@ public void propagate(int evtmask) throws ContradictionException {
                 }
             }
         }
-        for (int i = 0; i < n; i++) {
-            sdm[i].unfreeze();
-        }
     }
 
     @Override
     public void propagate(int idxVarInProp, int mask) throws ContradictionException {
         currentSet = idxVarInProp;
-        sdm[currentSet].freeze();
         sdm[currentSet].forEach(elementForced, SetEventType.ADD_TO_KER);
-        sdm[currentSet].unfreeze();
     }
 
     @Override

diff --git a/solver/src/main/java/org/chocosolver/solver/constraints/set/PropAllEqual.java b/solver/src/main/java/org/chocosolver/solver/constraints/set/PropAllEqual.java
@@ -107,17 +107,12 @@ public void propagate(int evtmask) throws ContradictionException {
                 }
             }
         }
-        for (int i = 0; i < n; i++) {
-            sdm[i].unfreeze();
-        }
     }
 
     @Override
     public void propagate(int idxVarInProp, int mask) throws ContradictionException {
-        sdm[idxVarInProp].freeze();
         sdm[idxVarInProp].forEach(elementForced, SetEventType.ADD_TO_KER);
         sdm[idxVarInProp].forEach(elementRemoved, SetEventType.REMOVE_FROM_ENVELOPE);
-        sdm[idxVarInProp].unfreeze();
     }
 
     @Override