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.

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