Skip to content

Add input validation for Friedman-Popescu H statistic calculation - #16760

Open
maurever with Copilot wants to merge 215 commits into
rel-3.46.0from
copilot/improve-input-validation
Open

Add input validation for Friedman-Popescu H statistic calculation#16760
maurever with Copilot wants to merge 215 commits into
rel-3.46.0from
copilot/improve-input-validation

Conversation

Copilot AI commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

Plan to Fix H Statistic Input Validation

  • Analyze the issue and understand the root cause
  • Add input validation in getFriedmanPopescusH() methods (XGBoostModel and GBMModel)
    • Validate vars is not null or empty
    • Validate all specified columns exist in the adapted frame
    • Validate all specified columns are numeric
  • Create test cases to validate the fix
  • Refactor to eliminate code duplication
  • Run code review and CodeQL checks
  • Change base branch to rel-3.46.0

Final Summary:

Root Cause

The NPE occurred when users specified column names in the vars parameter that:

  1. Don't exist in the frame, OR
  2. Are non-numeric (categorical) and get removed by removeSpecialNNonNumericColumns()

The error happened in FriedmanPopescusH.filterFrame() when calling frame.vecs(cols) with invalid column names, which returns null Vec references.

Solution Implemented

  1. Added comprehensive input validation in FriedmanPopescusHCollector interface as a default method
  2. Both GBM and XGBoost models now call this shared validation method
  3. Validation ensures all specified columns:
    • Are not null
    • Exist in the frame
    • Are numeric (not categorical)
    • Don't contain only missing or constant values
  4. Clear, actionable error messages guide users to fix their input

Test Coverage

  • Java: testFriedmanPopescusHInputValidation() in GBMTest
  • Python: pyunit_h_input_validation.py (tests both GBM and XGBoost)
  • R: runit_GBM_h_statistic_input_validation.R and runit_XGBoost_h_statistic_input_validation.R

Quality Checks

✅ Code compiles successfully
✅ Code review passed with no issues
✅ CodeQL security check passed
✅ No code duplication - shared validation method
✅ Base branch changed to rel-3.46.0 for inclusion in next release

Example Error Messages

Before Fix:

Server error java.lang.NullPointerException:
  Error: Cannot read field "_key" because "vecs[i]" is null

After Fix:

Calculating H statistics error: column 'fair' is not numeric. 
H statistics can only be calculated for numeric variables.
Original prompt

This section details on the original issue you should resolve

<issue_title>H statistic: Improve input validation</issue_title>
<issue_description>This code throws NPE:

from h2o.estimators import H2OXGBoostEstimator

# Import the titanic dataset into H2O:
titanic = h2o.import_file("https://s3.amazonaws.com/h2o-public-test-data/smalldata/gbm_test/titanic.csv")

# Set the predictors and response; set the response as a factor:
titanic["survived"] = titanic["survived"].asfactor()
predictors = titanic.columns
response = "survived"

# Split the dataset into a train and valid set:
train, valid = titanic.split_frame(ratios=[.8], seed=1234)

# Build and train the model:
titanic_xgb = H2OXGBoostEstimator(booster='dart',
                                  normalize_type="tree",
                                  seed=1234)
titanic_xgb.train(x=predictors,
                  y=response,
                  training_frame=train,
                  validation_frame=valid)

# Eval performance:
perf = titanic_xgb.model_performance()

# Generate predictions on a test set (if necessary):
pred = titanic_xgb.predict(valid)

# Extract feature interactions:
feature_interactions = titanic_xgb.feature_interaction()

# Get Friedman and Popescu's H statistics
h = titanic_xgb.h(train, ['fair','age'])
print(h)
Server error java.lang.NullPointerException:
  Error: Cannot read field "_key" because "vecs[i]" is null
  Request: None

The input parameter validation should be done properly. </issue_description>

<agent_instructions>Validate input parameters of the Friedman Popescus H statistics. On the Java Backend, find the getFriedmanPopescusH() method, where the implementation is for the XGBoost and GBM algorithms. </agent_instructions>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

CI user and others added 30 commits March 4, 2024 22:05
Improve custom metric doc
Co-authored-by: Hannah Tillman <hannah.tillman@h2o.ai>
tomasfryda and others added 27 commits May 26, 2026 09:55
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
…ity (#16803)

Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
…6811)

Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
…pe) (#16835)

* GH-16566 - Add per-test timeout rule to TestUtil

Adds a JUnit Timeout rule that fails individual tests with
TestTimedOutException instead of hanging the JVM. Default 10 minutes,
configurable via -Dtest.timeout.seconds. Prevents a single deadlocked
test from causing Jenkins to SIGTERM the entire stage.

* GH-16566 - Ignore flaky AdaBoost DeepLearning weak-learner test

testBasicTrainAndScoreDeepLearning hangs intermittently on Java 8/11
due to a model-lock race in DeepLearningDriver.onExceptionalCompletion:
the main thread blocks on a model whose FJ worker already threw.

* GH-16566 - Fix unmatched Scope.exit() in ParseTestOrc.testParseAllOrcs

The test called Scope.exit() in finally without a matching Scope.enter()
at the top, popping a scope it never pushed. It usually got rescued by
sibling tests leaking enters in the shared JVM; under newer JDK 17 test
ordering the donor scope is gone and the assertion fires.
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
…t grid-search hyperparameters

Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
…ctor' and 'model structure'

Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: Veronika Maurerová <maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
…80 (#16846)

Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
…ode test [nocheck] (#16813)

* WIP

* Fix non-determinism

* add assert
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
…026-42779 (#16853)

Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
Co-authored-by: maurever <11465784+maurever@users.noreply.github.com>
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
8.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

int inputColNum = cs.length;
int inputRowNum = cs[0]._len;
assert queryColNum == inputColNum: "Query data frame and input data frame should have the same columns number.";
for (int i = 0; i < queryRowNum; i++) { // go over all query data rows
* @return filled array of vecs with calculated data
*/
public Vec[] fillVecs(Vec[] vecs){
for (int i = 0; i < vecs[0].length(); i++) {
public static Vec createIdVec(final long len, byte type) {
assert type == Vec.T_UUID || type == Vec.T_STR || type == Vec.T_NUM: "Unsupported type for id vec creation: "+type;
final Vec vec = Vec.makeZero(len, type);
for (int i = 0; i < vec.length(); i++) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

H statistic: Improve input validation