Skip to content

XGRegressor::fit panics on a zero-row training set #446

Description

@teddytennant

XGRegressor::fit panics on a training set with zero rows, with default parameters and no subsample set. It hits the same line as #444 but the cause is different, so #445 does not fix it. I confirmed this panic still happens with #445 applied.

A zero-row matrix is reachable through the public Array2::take:

use smartcore::linalg::basic::matrix::DenseMatrix;
use smartcore::linalg::basic::arrays::{Array, Array2};
use smartcore::xgboost::{XGRegressor, XGRegressorParameters};

let full = DenseMatrix::from_2d_vec(&vec![vec![1.0, 1.0], vec![2.0, 1.0]]).unwrap();
let empty = full.take(&[] as &[usize], 0);
println!("shape = {:?}", empty.shape());   // (0, 2)

let y: Vec<f64> = vec![];
let _ = XGRegressor::fit(&empty, &y, XGRegressorParameters::default());
shape = (0, 2)
thread 'main' panicked at src/xgboost/xgb_regressor.rs:323:21:
attempt to subtract with overflow

find_best_split runs 0..sorted_idxs.len() - 1, which underflows on an empty slice whatever put it there. In release builds, where overflow checks are off, this comes out as index out of bounds: the len is 0 but the index is 0 instead.

The right behaviour is a design call, which is why I have not sent a patch. Either fit returns Failed::because(FailedError::ParametersError, ...) for empty training data, which matches the direction of #435, or it returns Ok with a degenerate model. I lean towards the error, since a model trained on no data is not useful, but it is your call and I am happy to send whichever you prefer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions