improved tests

Author: maurever

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

@@ -3416,7 +3416,7 @@ private void scoreAndUpdateModel() {
           _model._useRemoveOffsetEffects = false;
         }
       }
-      
+      _model.update(_job._key);
     }
 
     private void scorePostProcessingRestrictedModel(Frame train, long t1) {
@@ -3470,7 +3470,6 @@ private void scorePostProcessingRestrictedModel(Frame train, long t1) {
         _job.update(_workPerIteration, _state.toString());
       }
       _model._output._scoring_history = _scoringHistory != null ? _scoringHistory.to2dTable(_parms, null, null) : null;
-      _model.update(_job._key);
     }
 
     private void scorePostProcessingRestrictedModelCVEnabled(Frame train, long t1) {
@@ -3509,10 +3508,8 @@ private void scorePostProcessingRestrictedModelCVEnabled(Frame train, long t1) {
                   objectiveControlVal, _state.deviance(task._likelihood), Double.NaN, mtrain._nobs, 1, _state.lambda(),
                   _state.alpha());
 
-          _job.update(_workPerIteration, _state.toString());
         }
         _model._output._scoring_history_restricted_model_cv = _scoringHistoryControlValEnabled != null ? _scoringHistoryControlValEnabled.to2dTable(_parms, null, null) : null;
-        _model.update(_job._key);
       }
     }
 
@@ -3548,10 +3545,8 @@ private void scorePostProcessingRestrictedModelROEnabled(Frame train, long t1) {
                   _state.objective(), _state.deviance(), Double.NaN, mtrain._nobs, 1, _state.lambda(),
                   _state.alpha());
         }
-        _job.update(_workPerIteration, _state.toString());
       }
       _model._output._scoring_history_restricted_model_ro = _scoringHistoryRemoveOffsetEnabled != null ? _scoringHistoryRemoveOffsetEnabled.to2dTable(_parms, null, null) : null;
-      _model.update(_job._key);
     }
 
     private void scorePostProcessing(Frame train, long t1) {
@@ -3629,7 +3624,6 @@ private void scorePostProcessing(Frame train, long t1) {
                     _state.alpha());
           }
         }
-        _job.update(_workPerIteration, _state.toString());
       }
       if (_parms._lambda_search) {
         _model._output._scoring_history = _lambdaSearchScoringHistory.to2dTable();
@@ -3640,8 +3634,6 @@ private void scorePostProcessing(Frame train, long t1) {
         _model._output._scoring_history = _scoringHistory.to2dTable(_parms, _xval_deviances_generate_SH,
                 _xval_sd_generate_SH);
       }
-      
-      _model.update(_job._key);
       _model.generateSummary(_parms._train, _state._iter);
       _lastScore = System.currentTimeMillis();
       long scoringTime = System.currentTimeMillis() - t1;

h2o-bindings/bin/custom/R/gen_glm.py

@@ -80,8 +80,18 @@ def update_param(name, param):
 #'      this feature (available only if control_variables and remove_offset_effects parameters are both set)
 #' @export
 h2o.make_unrestricted_glm_model <- function(model, destination_key = NULL, control_variables_enabled = FALSE, remove_offset_effects_enabled = FALSE) {
-  stopifnot("GLM wasn't trained with control variables or with remove offset effects." = !is.null(model@params$actual[["control_variables"]]) || isTRUE(model@params$actual[["remove_offset_effects"]]))
-  query <- list(method = "POST", .h2o.__GLMMakeUnrestrictedModel, model = model@model_id, control_variables_enabled=control_variables_enabled, remove_offset_effects_enabled=remove_offset_effects_enabled)
+  stopifnot("GLM wasn't trained with control variables or with remove offset effects." = 
+    !is.null(model@params$actual[["control_variables"]]) || isTRUE(model@params$actual[["remove_offset_effects"]]))
+  if (is.null(model@params$actual[["control_variables"]]) && isFALSE(model@params$actual[["remove_offset_effects"]]) 
+      && (isTRUE(control_variables_enabled) || isTRUE(remove_offset_effects_enabled))) { 
+       stop("GLM wasn't trained with both control variables and with remove offset effects feature set, the control_variables_enabled and remove_offset_effects_enabled features cannot be used.")  
+  }
+  if ((!is.null(model@params$actual[["control_variables"]]) || isTRUE(model@params$actual[["remove_offset_effects"]]))
+      && (isTRUE(control_variables_enabled) && isTRUE(remove_offset_effects_enabled))){
+      stop("The control_variables_enabled and remove_offset_effects_enabled feature cannot be used together. It produces the same model as the main model.")
+  }
+  query <- list(method = "POST", .h2o.__GLMMakeUnrestrictedModel, model = model@model_id, 
+    control_variables_enabled=control_variables_enabled, remove_offset_effects_enabled=remove_offset_effects_enabled)
   if (!missing(destination_key) && !is.null(destination_key)) {
     query <- c(query, list(dest = destination_key))
   }

h2o-bindings/bin/custom/python/gen_glm.py

@@ -335,10 +335,16 @@ def make_unrestricted_glm_model(self, dest=None, control_variables_enabled=False
         >>> m2 = m.make_unrestricted_glm_model(dest="unrestricted_glm")
         >>> p2 = m2.model_performance(d)
         >>> print(p2)
-        >>> m3 = m.make_unrestricted_glm_model(dest="unrestricted_glm", control_variables_enabled=True)
+        >>> m3 = m.make_unrestricted_glm_model(dest="unrestricted_glm_cv", control_variables_enabled=True)
         >>> p3 = m3.model_performance(d)
         >>> print(p3)
         """
+        if self.actual_params["control_variables"] is None and not(self.actual_params["remove_offset_effects"]):
+            raise H2OValueError("GLM wasn't trained with control variables or with remove offset effects.")
+        if (self.actual_params["control_variables"] is None or not(self.actual_params["remove_offset_effects"])) and (control_variables_enabled or remove_offset_effects_enabled):
+            raise H2OValueError("GLM wasn't trained with both control variables and with remove offset effects feature set, the control_variables_enabled and remove_offset_effects_enabled features cannot be used.")
+        if self.actual_params["control_variables"] is not None and self.actual_params["remove_offset_effects"] and (control_variables_enabled and remove_offset_effects_enabled):
+            raise H2OValueError("The control_variables_enabled and remove_offset_effects_enabled feature cannot be used together. It produces the same model as the main model.")
         model_json = h2o.api(
             "POST /3/MakeUnrestrictedGLMModel",
             data={"model": self._model_json["model_id"]["name"],

h2o-py/h2o/estimators/glm.py

@@ -2848,10 +2848,16 @@ def make_unrestricted_glm_model(self, dest=None, control_variables_enabled=False
         >>> m2 = m.make_unrestricted_glm_model(dest="unrestricted_glm")
         >>> p2 = m2.model_performance(d)
         >>> print(p2)
-        >>> m3 = m.make_unrestricted_glm_model(dest="unrestricted_glm", control_variables_enabled=True)
+        >>> m3 = m.make_unrestricted_glm_model(dest="unrestricted_glm_cv", control_variables_enabled=True)
         >>> p3 = m3.model_performance(d)
         >>> print(p3)
         """
+        if self.actual_params["control_variables"] is None and not(self.actual_params["remove_offset_effects"]):
+            raise H2OValueError("GLM wasn't trained with control variables or with remove offset effects.")
+        if (self.actual_params["control_variables"] is None or not(self.actual_params["remove_offset_effects"])) and (control_variables_enabled or remove_offset_effects_enabled):
+            raise H2OValueError("GLM wasn't trained with both control variables and with remove offset effects feature set, the control_variables_enabled and remove_offset_effects_enabled features cannot be used.")
+        if self.actual_params["control_variables"] is not None and self.actual_params["remove_offset_effects"] and (control_variables_enabled and remove_offset_effects_enabled):
+            raise H2OValueError("The control_variables_enabled and remove_offset_effects_enabled feature cannot be used together. It produces the same model as the main model.")
         model_json = h2o.api(
             "POST /3/MakeUnrestrictedGLMModel",
             data={"model": self._model_json["model_id"]["name"],

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

@@ -2,33 +2,32 @@
 
 from h2o.exceptions import H2OResponseError
 
-sys.path.insert(1,"../../../")
+sys.path.insert(1, "../../../")
 import h2o
 from tests import pyunit_utils
 from h2o.estimators.glm import H2OGeneralizedLinearEstimator
 
 
 def glm_unrestricted_model():
-
     cars = h2o.upload_file(pyunit_utils.locate("smalldata/junit/cars_20mpg.csv"))
     cars = cars[cars["economy_20mpg"].isna() == 0]
     cars["name"] = cars["name"].asfactor()
     cars["economy_20mpg"] = cars["economy_20mpg"].asfactor()
     offset_col = "offset"
-    offset = h2o.H2OFrame([[.5]]*398)
+    offset = h2o.H2OFrame([[.5]] * 398)
     offset.set_names([offset_col])
     cars = cars.cbind(offset)
 
     print("-- Model without control variables and remove offset effects --")
-    glm_model = H2OGeneralizedLinearEstimator(family="binomial", score_each_iteration=True, 
+    glm_model = H2OGeneralizedLinearEstimator(family="binomial", score_each_iteration=True,
                                               generate_scoring_history=True, seed=0xC0FFEE)
     glm_model.train(x=["name", "power", "year"], y="economy_20mpg", training_frame=cars, offset_column=offset_col)
     metrics = glm_model.training_model_metrics()
     print(metrics)
     print(glm_model)
 
     print("-- Model with control variables --")
-    glm_model_cv = H2OGeneralizedLinearEstimator(family="binomial", control_variables=["year"], 
+    glm_model_cv = H2OGeneralizedLinearEstimator(family="binomial", control_variables=["year"],
                                                  score_each_iteration=True, generate_scoring_history=True,
                                                  seed=0xC0FFEE)
     glm_model_cv.train(x=["name", "power", "year"], y="economy_20mpg", training_frame=cars, offset_column=offset_col)
@@ -43,7 +42,7 @@ def glm_unrestricted_model():
     print(metrics_unrestricted_cv)
 
     print("-- Model with remove offset effects --")
-    glm_model_ro = H2OGeneralizedLinearEstimator(family="binomial", remove_offset_effects=True, 
+    glm_model_ro = H2OGeneralizedLinearEstimator(family="binomial", remove_offset_effects=True,
                                                  generate_scoring_history=True,
                                                  score_each_iteration=True, seed=0xC0FFEE)
     glm_model_ro.train(x=["name", "power", "year"], y="economy_20mpg", training_frame=cars, offset_column=offset_col)
@@ -58,7 +57,7 @@ def glm_unrestricted_model():
     print(metrics_unrestricted_ro)
 
     print("-- Model with control variables and remove offset effects --")
-    glm_model_cv_ro = H2OGeneralizedLinearEstimator(family="binomial", control_variables=["year"], 
+    glm_model_cv_ro = H2OGeneralizedLinearEstimator(family="binomial", control_variables=["year"],
                                                     remove_offset_effects=True, generate_scoring_history=True,
                                                     score_each_iteration=True, seed=0xC0FFEE)
     glm_model_cv_ro.train(x=["name", "power", "year"], y="economy_20mpg", training_frame=cars, offset_column=offset_col)
@@ -73,7 +72,7 @@ def glm_unrestricted_model():
     print(metrics_unrestricted_cv_ro)
 
     print("-- Unrestricted model with control variables enabled and remove offset effects disabled --")
-    glm_model_unrestricted_cv_true_ro_false = glm_model_cv_ro.make_unrestricted_glm_model(dest="cv_true", 
+    glm_model_unrestricted_cv_true_ro_false = glm_model_cv_ro.make_unrestricted_glm_model(dest="cv_true",
                                                                                           control_variables_enabled=True)
     print(glm_model_unrestricted_cv_ro)
     metrics_unrestricted_cv_true_ro_false = glm_model_unrestricted_cv_true_ro_false.training_model_metrics()
@@ -107,36 +106,67 @@ def glm_unrestricted_model():
 
     # check the coefficients
     for k in glm_model.coef().keys():
-        pyunit_utils.assert_equals(glm_model.coef()[k], glm_model_unrestricted_cv.coef().get(k, float("NaN")), f"Coefficient {k} differs!")
+        pyunit_utils.assert_equals(glm_model.coef()[k], glm_model_unrestricted_cv.coef().get(k, float("NaN")),
+                                   f"Coefficient {k} differs!")
 
     # check predictions are the same
     for i in range(predictions.shape[0]):
-        pyunit_utils.assert_equals(predictions.iloc[i, 1], predictions_unrestricted_cv.iloc[i, 1], f"{i}th prediction differs!")
-        pyunit_utils.assert_equals(predictions.iloc[i, 1], predictions_unrestricted_ro.iloc[i, 1], f"{i}th prediction differs!")
-        pyunit_utils.assert_equals(predictions.iloc[i, 1], predictions_unrestricted_cv_ro.iloc[i, 1], f"{i}th prediction differs!")
-        pyunit_utils.assert_equals(predictions_cv.iloc[i, 1], predictions_unrestricted_cv_true_ro_false.iloc[i, 1], f"{i}th prediction differs!")
-        pyunit_utils.assert_equals(predictions_ro.iloc[i, 1], predictions_unrestricted_cv_false_ro_true.iloc[i, 1], f"{i}th prediction differs!")
-        
+        pyunit_utils.assert_equals(predictions.iloc[i, 1], predictions_unrestricted_cv.iloc[i, 1],
+                                   f"{i}th prediction differs!")
+        pyunit_utils.assert_equals(predictions.iloc[i, 1], predictions_unrestricted_ro.iloc[i, 1],
+                                   f"{i}th prediction differs!")
+        pyunit_utils.assert_equals(predictions.iloc[i, 1], predictions_unrestricted_cv_ro.iloc[i, 1],
+                                   f"{i}th prediction differs!")
+        pyunit_utils.assert_equals(predictions_cv.iloc[i, 1], predictions_unrestricted_cv_true_ro_false.iloc[i, 1],
+                                   f"{i}th prediction differs!")
+        pyunit_utils.assert_equals(predictions_ro.iloc[i, 1], predictions_unrestricted_cv_false_ro_true.iloc[i, 1],
+                                   f"{i}th prediction differs!")
+
     # check predictions differ
     for i in range(predictions.shape[0]):
-        pyunit_utils.assert_not_equal(predictions.iloc[i, 1], predictions_cv.iloc[i, 1], f"Predictions at position {i} should differ but they don't!")
-        pyunit_utils.assert_not_equal(predictions.iloc[i, 1], predictions_ro.iloc[i, 1], f"Predictions at position {i} should differ but they don't!")
-        pyunit_utils.assert_not_equal(predictions.iloc[i, 1], predictions_cv_ro.iloc[i, 1], f"Predictions at position {i} should differ but they don't!")
-        pyunit_utils.assert_not_equal(predictions_unrestricted_cv_false_ro_true.iloc[i, 1], predictions_unrestricted_cv_true_ro_false.iloc[i, 1], f"Predictions at position {i} should differ but they don't!")
-        
+        pyunit_utils.assert_not_equal(predictions.iloc[i, 1], predictions_cv.iloc[i, 1],
+                                      f"Predictions at position {i} should differ but they don't!")
+        pyunit_utils.assert_not_equal(predictions.iloc[i, 1], predictions_ro.iloc[i, 1],
+                                      f"Predictions at position {i} should differ but they don't!")
+        pyunit_utils.assert_not_equal(predictions.iloc[i, 1], predictions_cv_ro.iloc[i, 1],
+                                      f"Predictions at position {i} should differ but they don't!")
+        pyunit_utils.assert_not_equal(predictions_unrestricted_cv_false_ro_true.iloc[i, 1],
+                                      predictions_unrestricted_cv_true_ro_false.iloc[i, 1],
+                                      f"Predictions at position {i} should differ but they don't!")
+
     print(glm_model_cv.scoring_history())
     print(glm_model_unrestricted_cv_true_ro_false.scoring_history())
-    
+
     # check scoring history are the same
-    pyunit_utils.assert_equal_scoring_history(glm_model, glm_model_unrestricted_cv, 
+    pyunit_utils.assert_equal_scoring_history(glm_model, glm_model_unrestricted_cv,
                                               ["objective", "negative_log_likelihood"])
-    pyunit_utils.assert_equal_scoring_history(glm_model_cv, glm_model_unrestricted_cv_true_ro_false, 
+    pyunit_utils.assert_equal_scoring_history(glm_model_cv, glm_model_unrestricted_cv_true_ro_false,
                                               ["objective", "negative_log_likelihood", "deviance_train", "lambda"])
     pyunit_utils.assert_equal_scoring_history(glm_model_ro, glm_model_unrestricted_cv_false_ro_true,
                                               ["objective", "negative_log_likelihood", "deviance_train", "lambda"])
     pyunit_utils.assert_equal_scoring_history(glm_model_unrestricted_cv, glm_model_unrestricted_cv_ro,
                                               ["objective", "negative_log_likelihood", "deviance_train", "lambda"])
 
+    # should fail
+    try:
+        glm_model_ro.make_unrestricted_glm_model(dest="ro_true", remove_offset_effects_enabled=True)
+        assert False, "Should have throw exception."
+    except Exception as ex:
+        print(ex)
+        temp = str(ex)
+        assert "GLM wasn't trained with both control variables and with remove offset effects feature set, the control_variables_enabled and remove_offset_effects_enabled features cannot be used." in temp, \
+            "Wrong exception was received."
+
+    try:
+        glm_model_cv_ro.make_unrestricted_glm_model(dest="ro_true", remove_offset_effects_enabled=True,
+                                                    control_variables_enabled=True)
+        assert False, "Should have throw exception."
+    except Exception as ex:
+        print(ex)
+        temp = str(ex)
+        assert "The control_variables_enabled and remove_offset_effects_enabled feature cannot be used together. It produces the same model as the main model." in temp, \
+            "Wrong exception was received."
+
 
 if __name__ == "__main__":
     pyunit_utils.standalone_test(glm_unrestricted_model)

h2o-r/h2o-package/R/glm.R

@@ -843,8 +843,18 @@ h2o.makeGLMModel <- function(model,beta) {
 #'      this feature (available only if control_variables and remove_offset_effects parameters are both set)
 #' @export
 h2o.make_unrestricted_glm_model <- function(model, destination_key = NULL, control_variables_enabled = FALSE, remove_offset_effects_enabled = FALSE) {
-  stopifnot("GLM wasn't trained with control variables or with remove offset effects." = !is.null(model@params$actual[["control_variables"]]) || isTRUE(model@params$actual[["remove_offset_effects"]]))
-  query <- list(method = "POST", .h2o.__GLMMakeUnrestrictedModel, model = model@model_id, control_variables_enabled=control_variables_enabled, remove_offset_effects_enabled=remove_offset_effects_enabled)
+  stopifnot("GLM wasn't trained with control variables or with remove offset effects." = 
+    !is.null(model@params$actual[["control_variables"]]) || isTRUE(model@params$actual[["remove_offset_effects"]]))
+  if (is.null(model@params$actual[["control_variables"]]) && isFALSE(model@params$actual[["remove_offset_effects"]]) 
+      && (isTRUE(control_variables_enabled) || isTRUE(remove_offset_effects_enabled))) { 
+       stop("GLM wasn't trained with both control variables and with remove offset effects feature set, the control_variables_enabled and remove_offset_effects_enabled features cannot be used.")  
+  }
+  if ((!is.null(model@params$actual[["control_variables"]]) || isTRUE(model@params$actual[["remove_offset_effects"]]))
+      && (isTRUE(control_variables_enabled) && isTRUE(remove_offset_effects_enabled))){
+      stop("The control_variables_enabled and remove_offset_effects_enabled feature cannot be used together. It produces the same model as the main model.")
+  }
+  query <- list(method = "POST", .h2o.__GLMMakeUnrestrictedModel, model = model@model_id, 
+    control_variables_enabled=control_variables_enabled, remove_offset_effects_enabled=remove_offset_effects_enabled)
   if (!missing(destination_key) && !is.null(destination_key)) {
     query <- c(query, list(dest = destination_key))
   }