Fix bug in adding in beta constraints twice in checking for constrain…

…t matrix rank.

h2o-algos/src/main/java/hex/glm/ConstrainedGLMUtils.java

@@ -6,6 +6,7 @@
 import water.Iced;
 import water.Key;
 import water.fvec.Frame;
+import water.util.ArrayUtils;
 import water.util.IcedHashMap;
 
 import java.util.*;
@@ -116,16 +117,18 @@ public static LinearConstraints[] combineConstraints(LinearConstraints[] const1,
    * 1): -Infinity <= beta <= Infinity: ignored, no constrain here;
    * 2): -Infinity <= beta <= high_val: transformed to beta - high_val <= 0, add to lessThanEqualTo constraint;
    * 3): low_val <= beta <= Infinity: transformed to low_val - beta <= 0, add to lessThanEqualTo constraint;
-   * 4): low_val <= beta <= high_val: transformed to two constraints, low_val-beta <= 0, beta-high_val <= 0, add to lessThanEqualTo constraint;
+   * 4): low_val <= beta <= high_val: transformed to two constraints, low_val-beta <= 0, beta-high_val <= 0, add to 
+   *   lessThanEqualTo constraint.
    * 5): val <= beta <= val: transformed to beta-val == 0, add to equalTo constraint.
    * 
    * The newly extracted constraints will be added to fields in state.
    * 
    */
-  public static void extractBetaConstraints(ComputationState state, String[] coefNames) {
+  public static int[] extractBetaConstraints(ComputationState state, String[] coefNames) {
     GLM.BetaConstraint betaC = state.activeBC();
     List<LinearConstraints> equalityC = new ArrayList<>();
     List<LinearConstraints> lessThanEqualToC = new ArrayList<>();
+    List<Integer> betaLessThanC = new ArrayList<>();
     if (betaC._betaLB != null) {
       int numCons = betaC._betaLB.length-1;
       for (int index=0; index<numCons; index++) {
@@ -135,15 +138,20 @@ public static void extractBetaConstraints(ComputationState state, String[] coefN
                 (betaC._betaLB[index] < betaC._betaUB[index])) { // low < beta < high, generate two lessThanEqualTo constraints
           addBCGreaterThanConstraint(lessThanEqualToC, betaC, coefNames, index);
           addBCLessThanConstraint(lessThanEqualToC, betaC, coefNames, index);
+          betaLessThanC.add(1);
+          betaLessThanC.add(0);
         } else if (Double.isInfinite(betaC._betaUB[index]) && !Double.isInfinite(betaC._betaLB[index])) {  // low < beta < infinity
           addBCGreaterThanConstraint(lessThanEqualToC, betaC, coefNames, index);
+          betaLessThanC.add(1);
         } else if (!Double.isInfinite(betaC._betaUB[index]) && Double.isInfinite(betaC._betaLB[index])) { // -infinity < beta < high
           addBCLessThanConstraint(lessThanEqualToC, betaC, coefNames, index);
+          betaLessThanC.add(1);
         }
       }
     }
     state.setLinearConstraints(equalityC.toArray(new LinearConstraints[0]),
             lessThanEqualToC.toArray(new LinearConstraints[0]), true);
+    return betaLessThanC.size()==0 ? null : betaLessThanC.stream().mapToInt(x->x).toArray();
   }
 
   /***
@@ -263,40 +271,46 @@ public static void extractConstraint(Frame constraintF, List<Integer> rowIndices
     rowIndices.removeAll(processedRowIndices);
   }
 
-  public static double[][] formConstraintMatrix(ComputationState state, List<String> constraintNamesList) {
+  public static double[][] formConstraintMatrix(ComputationState state, List<String> constraintNamesList, int[] betaLessThan) {
     // extract coefficient names from constraints
     constraintNamesList.addAll(extractConstraintCoeffs(state));
     // form double matrix
     int numRow = (state._equalityConstraintsBeta == null ? 0 : state._equalityConstraintsBeta.length) +
-            (state._lessThanEqualToConstraintsBeta == null ? 0 : state._lessThanEqualToConstraintsBeta.length) +
+            (state._lessThanEqualToConstraintsBeta == null ? 0 : (betaLessThan == null ? 0 : ArrayUtils.sum(betaLessThan))) +
             (state._equalityConstraints == null ? 0 : state._equalityConstraints.length) + 
             (state._lessThanEqualToConstraints == null ? 0 : state._lessThanEqualToConstraints.length);
     double[][] initConstraintMatrix = new double[numRow][constraintNamesList.size()];
-    fillConstraintValues(state, constraintNamesList, initConstraintMatrix);
+    fillConstraintValues(state, constraintNamesList, initConstraintMatrix, betaLessThan);
     return initConstraintMatrix;
   }
 
-  public static void fillConstraintValues(ComputationState state, List<String> constraintNamesList, double[][] initCMatrix) {
+  public static void fillConstraintValues(ComputationState state, List<String> constraintNamesList, 
+                                          double[][] initCMatrix, int[] betaLessThan) {
     int rowIndex = 0;
     if (state._equalityConstraintsBeta != null)
-      rowIndex = extractConstraintValues(state._equalityConstraintsBeta, constraintNamesList, initCMatrix, rowIndex);
+      rowIndex = extractConstraintValues(state._equalityConstraintsBeta, constraintNamesList, initCMatrix, rowIndex, 
+              null);
     if (state._lessThanEqualToConstraintsBeta != null)
-      rowIndex= extractConstraintValues(state._lessThanEqualToConstraintsBeta, constraintNamesList, initCMatrix, rowIndex);
+      rowIndex= extractConstraintValues(state._lessThanEqualToConstraintsBeta, constraintNamesList, initCMatrix, 
+              rowIndex, betaLessThan);
     if (state._equalityConstraints != null)
-      rowIndex = extractConstraintValues(state._equalityConstraints, constraintNamesList, initCMatrix, rowIndex);
+      rowIndex = extractConstraintValues(state._equalityConstraints, constraintNamesList, initCMatrix, rowIndex, null);
     if (state._lessThanEqualToConstraints != null)
-      extractConstraintValues(state._lessThanEqualToConstraints, constraintNamesList, initCMatrix, rowIndex);
+      extractConstraintValues(state._lessThanEqualToConstraints, constraintNamesList, initCMatrix, rowIndex, null);
   }
 
-  public static int extractConstraintValues(LinearConstraints[] constraints, List<String> constraintNamesList, double[][] initCMatrix, int rowIndex) {
+  public static int extractConstraintValues(LinearConstraints[] constraints, List<String> constraintNamesList, 
+                                            double[][] initCMatrix, int rowIndex, int[] betaLessThan) {
     int numConstr = constraints.length;
     for (int index=0; index<numConstr; index++) {
-      Set<String> coeffKeys = constraints[index]._constraints.keySet();
-      for (String oneKey : coeffKeys) {
-        if ( constraintNamesList.contains(oneKey))
-          initCMatrix[rowIndex][constraintNamesList.indexOf(oneKey)] = constraints[index]._constraints.get(oneKey);
+      if (betaLessThan == null || betaLessThan[index] == 1) {
+        Set<String> coeffKeys = constraints[index]._constraints.keySet();
+        for (String oneKey : coeffKeys) {
+          if (constraintNamesList.contains(oneKey))
+            initCMatrix[rowIndex][constraintNamesList.indexOf(oneKey)] = constraints[index]._constraints.get(oneKey);
+        }
+        rowIndex++;
       }
-      rowIndex++;
     }
     return rowIndex;
   }

h2o-algos/src/main/java/hex/glm/GLM.java

@@ -1233,7 +1233,7 @@ public void init(boolean expensive) {
           } else {
             System.arraycopy(_parms._startval, 0, beta, 0, beta.length);
           }
-        } else if (_parms._linear_constraints != null) {  // start value is not assigned
+        } else if (_parms._linear_constraints != null && _parms._init_optimal_glm) {  // start value is not assigned
           beta = genInitBeta();
         }
 
@@ -1400,13 +1400,14 @@ void checkAssignLinearConstraints() {
       return;
     }
     String[] coefNames = _dinfo.coefNames();
+    int[] betaLessThanArr = null;
     if (_parms._beta_constraints != null)
-      extractBetaConstraints(_state, coefNames);
+      betaLessThanArr = extractBetaConstraints(_state, coefNames);
     // extract constraints from linear_constraints into equality of lessthanequalto constraints
     extractLinearConstraints(_state, _parms._linear_constraints, _dinfo);
     // make sure constraints have full rank.  If not, generate messages stating what constraints are redundant, error out
     List<String> constraintNames = new ArrayList<>();
-    double[][] initConstraintMatrix = formConstraintMatrix(_state, constraintNames);
+    double[][] initConstraintMatrix = formConstraintMatrix(_state, constraintNames, betaLessThanArr);
     String[] constraintCoefficientNames = constraintNames.toArray(new String[0]);
     if (countNumConst(_state) > coefNames.length)
       warn("number of constraints", " exceeds the number of coefficients.  The system is" +

h2o-algos/src/main/java/hex/glm/GLMModel.java

@@ -539,7 +539,7 @@ public enum Constraints {EqualTo, LessThanEqualTo};
     public boolean _keepBetaDiffVar = false;  // if true, will keep the frame generating the beta without from i and the variance estimation
     boolean _testCSZeroGram = false;  // internal parameter, to test zero gram dropped column is correctly implemented
     public boolean _separate_linear_beta = false;  // if true, will perform the beta and linear constraint separately
-    public boolean _init_optimal_glm = true;  // only used when there is linear constraints
+    public boolean _init_optimal_glm = false;  // only used when there is linear constraints
 
     public void validate(GLM glm) {
       if (_remove_collinear_columns) {

h2o-algos/src/main/java/hex/schemas/GLMV3.java

@@ -362,7 +362,7 @@ public static final class GLMParametersV3 extends ModelParametersSchemaV3<GLMPar
                                               // scoring_iteration_interval and score_every_iteration
 
     @API(help="If true, will initialize coefficients with values derived from GLM runs without linear constraints.  " +
-            "Only available for linear constraints.  Default to true.", level = API.Level.secondary, 
+            "Only available for linear constraints.  Default to false.", level = API.Level.secondary, 
             direction = API.Direction.INOUT, gridable = true)
     public boolean init_optimal_glm;
 

h2o-py/h2o/estimators/glm.py

@@ -118,7 +118,7 @@ def __init__(self,
                  influence=None,  # type: Optional[Literal["dfbetas"]]
                  gainslift_bins=-1,  # type: int
                  linear_constraints=None,  # type: Optional[Union[None, str, H2OFrame]]
-                 init_optimal_glm=True,  # type: bool
+                 init_optimal_glm=False,  # type: bool
                  separate_linear_beta=False,  # type: bool
                  ):
         """
@@ -433,8 +433,8 @@ def __init__(self,
                Defaults to ``None``.
         :type linear_constraints: Union[None, str, H2OFrame], optional
         :param init_optimal_glm: If true, will initialize coefficients with values derived from GLM runs without linear
-               constraints.  Only available for linear constraints.  Default to true.
-               Defaults to ``True``.
+               constraints.  Only available for linear constraints.  Default to false.
+               Defaults to ``False``.
         :type init_optimal_glm: bool
         :param separate_linear_beta: If true, will keep the beta constraints and linear constraints separate.  After new
                coefficients arefound, first beta constraints will be applied followed by the application of linear
@@ -2464,9 +2464,9 @@ def linear_constraints(self, linear_constraints):
     def init_optimal_glm(self):
         """
         If true, will initialize coefficients with values derived from GLM runs without linear constraints.  Only
-        available for linear constraints.  Default to true.
+        available for linear constraints.  Default to false.
 
-        Type: ``bool``, defaults to ``True``.
+        Type: ``bool``, defaults to ``False``.
         """
         return self._parms.get("init_optimal_glm")
 

h2o-py/tests/testdir_algos/glm/pyunit_GH_6722_loose_beta_linear_constraints_binomial.py

@@ -17,9 +17,9 @@ def test_constraints_binomial():
                                             lambda_=0.0, solver="irlsm", seed=12345, standardize=True)
     h2o_glm.train(x=predictors, y=response, training_frame=train)
     logloss = h2o_glm.model_performance()._metric_json['logloss']
+    print("logloss with no constraints: {0}".format(logloss))
 
     bc = []
-
     name = "C11"
     lower_bound = -8
     upper_bound = 0
@@ -42,8 +42,7 @@ def test_constraints_binomial():
 
     beta_constraints = h2o.H2OFrame(bc)
     beta_constraints.set_names(["names", "lower_bounds", "upper_bounds"])
-
-
+
     # add loose constraints
     name = "C19"
     values = 0.5
@@ -87,31 +86,37 @@ def test_constraints_binomial():
     h2o_glm_optimal_init = H2OGeneralizedLinearEstimator(family="binomial", compute_p_values=True, lambda_=0.0, 
                                                          seed=12345, remove_collinear_columns=True,solver="irlsm", 
                                                          linear_constraints=linear_constraints2, 
-                                                         beta_constraints=beta_constraints)
+                                                         beta_constraints=beta_constraints, init_optimal_glm=True)
     h2o_glm_optimal_init.train(x=predictors, y=response, training_frame=train)
     init_logloss = h2o_glm_optimal_init.model_performance()._metric_json['logloss']
+    print("logloss with constraints and coefficients initialized with glm model built without constraints: {0}".format(init_logloss))
     # GLM model with GLM coefficients with default initialization
     h2o_glm_default_init = H2OGeneralizedLinearEstimator(family="binomial", compute_p_values=True, lambda_=0.0,
                                                          seed=12345, remove_collinear_columns=True,solver="irlsm",
                                                          linear_constraints=linear_constraints2,
                                                          beta_constraints=beta_constraints, init_optimal_glm = False)
     h2o_glm_default_init.train(x=predictors, y=response, training_frame=train)
     default_init_logloss = h2o_glm_default_init.model_performance()._metric_json['logloss']
+    print("logloss with constraints and default coefficients initialization: {0}".format(default_init_logloss))
     # GLM with beta/linear constraints separate, default coef init
     h2o_glm_optimal_init_sep = H2OGeneralizedLinearEstimator(family="binomial", compute_p_values=True, lambda_=0.0,
                                                          seed=12345, remove_collinear_columns=True,solver="irlsm",
                                                          linear_constraints=linear_constraints2,
-                                                         beta_constraints=beta_constraints, separate_linear_beta=True)
+                                                         beta_constraints=beta_constraints, separate_linear_beta=True,
+                                                             init_optimal_glm=True)
     h2o_glm_optimal_init_sep.train(x=predictors, y=response, training_frame=train)
     init_logloss_sep = h2o_glm_optimal_init_sep.model_performance()._metric_json['logloss']
+    print("logloss with constraints and coefficients initialized with glm model built without constraints, beta and"
+          " linear constraints are applied separately: {0}".format(init_logloss_sep))
     # GLM with beta/linear constraints separate, optimal coef init
     h2o_glm_default_init_sep = H2OGeneralizedLinearEstimator(family="binomial", compute_p_values=True, lambda_=0.0,
                                                          seed=12345, remove_collinear_columns=True,solver="irlsm",
                                                          linear_constraints=linear_constraints2,
                                                          beta_constraints=beta_constraints, init_optimal_glm = False)
     h2o_glm_default_init_sep.train(x=predictors, y=response, training_frame=train)
     default_init_logloss_sep = h2o_glm_default_init_sep.model_performance()._metric_json['logloss']
-
+    print("logloss with constraints and default coefficients initialization, beta and linear constraints are applied"
+          " separately: {0}".format(default_init_logloss_sep))
     # since the constraints are loose, performance of GLM model without linear constraints and GLM model with linear
     # constraint and initialized with optimal GLM model coefficients should equal.  We will compare the logloss
     assert abs(logloss-init_logloss)<1e-6, "logloss from optimal GLM {0} and logloss from GLM with loose constraints " \
@@ -122,6 +127,18 @@ def test_constraints_binomial():
                                             "and with default initial coefficients {1} but is" \
                                             " not.".format(logloss, default_init_logloss)
 
+    # logloss without constraint should be better than logloss with constraints
+    assert logloss <= init_logloss_sep, "logloss from optimal GLM {0} should be better than GLM with constraints " \
+                                            "and with default initial coefficients and separating beta and linear " \
+                                                "constraints {1} but is" \
+                                            " not.".format(logloss, default_init_logloss)
+
+    # logloss without constraint should be better than logloss with constraints
+    assert logloss <= default_init_logloss_sep, "logloss from optimal GLM {0} should be better than GLM with constraints " \
+                                            "and with default initial coefficients and separating beta and linear " \
+                                                "constraints {1} but is" \
+                                            " not.".format(logloss, default_init_logloss)
+
 if __name__ == "__main__":
     pyunit_utils.standalone_test(test_constraints_binomial)
 else:

h2o-py/tests/testdir_algos/glm/pyunit_GH_6722_loose_only_linear_constraints_binomial.py

@@ -58,8 +58,10 @@ def test_constraints_binomial():
     linear_constraints2 = h2o.H2OFrame(loose_init_const)
     linear_constraints2.set_names(["names", "values", "types", "constraint_numbers"])    
     # GLM model with with GLM coefficients set to GLM model coefficients built without constraints
-    h2o_glm_optimal_init = H2OGeneralizedLinearEstimator(family="binomial", compute_p_values=True, remove_collinear_columns=True,
-                                            lambda_=0.0, solver="irlsm", linear_constraints=linear_constraints2, seed=12345)
+    h2o_glm_optimal_init = H2OGeneralizedLinearEstimator(family="binomial", compute_p_values=True, 
+                                                         remove_collinear_columns=True, lambda_=0.0, solver="irlsm", 
+                                                         linear_constraints=linear_constraints2, seed=12345,
+                                                         init_optimal_glm=True)
     h2o_glm_optimal_init.train(x=predictors, y=response, training_frame=train)
     init_logloss = h2o_glm_optimal_init.model_performance()._metric_json['logloss']
     # GLM model with GLM coefficients with default initialization