diff --git a/CHANGELOG.md b/CHANGELOG.md index 62b0791a..26b52ccc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.6.12] +### Fixed +- `xgboost/xgb_regressor.rs`: `XGRegressor::fit` no longer panics with `attempt to subtract with overflow` when `subsample` is less than 1.0 on a small dataset (#444). `floor(n_samples * subsample)` truncates to 0 rows. This occurs with 3 rows at a ratio of 0.3, or with 1 row at any ratio below 1.0. The tree fit then read `sorted_idxs.len() - 1` on an empty index set. The sample size now keeps a minimum of one row, as scikit-learn does for its own `subsample` parameter. Sample sizes of one row or more are unchanged. + ## [0.6.11] ### Fixed - `algorithm/neighbour/cosinepair.rs`: `CosinePair::query_row_top_k` now returns exact nearest neighbours whenever `approximate` is `false` (the default). Previously the query always sampled only `top_k` evenly strided candidate rows without documentation, and the bounded candidate heap evicted its closest entry, so the method could return the farthest of the sampled rows (#442). Strided sampling is now gated behind `CosinePairParameters { approximate: true, .. }` and is documented as approximate. diff --git a/Cargo.toml b/Cargo.toml index b36a8c96..bdbb9135 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "smartcore" description = "Machine Learning in Rust." homepage = "https://smartcorelib.github.io/" -version = "0.6.11" +version = "0.6.12" authors = ["smartcore Developers"] edition = "2024" rust-version = "1.85" diff --git a/src/xgboost/xgb_regressor.rs b/src/xgboost/xgb_regressor.rs index 37aa7e2e..168c21d0 100644 --- a/src/xgboost/xgb_regressor.rs +++ b/src/xgboost/xgb_regressor.rs @@ -491,6 +491,8 @@ impl XGRegressorParameters { /// Sets the fraction of samples to be used for fitting individual base learners. /// /// A value of less than 1.0 introduces randomness and helps prevent overfitting. + /// The value must be in the range (0, 1]. Each tree gets `floor(n_samples * subsample)` + /// rows, but a minimum of one row. pub fn with_subsample(mut self, subsample: f64) -> Self { self.subsample = subsample; self @@ -599,6 +601,9 @@ impl, Y: Array1> XGRegres } /// Creates a random sample of indices without replacement. + /// + /// The sample holds at least one index when the population is not empty. The tree fit + /// needs a minimum of one row, thus the sample size cannot be zero. fn sample_without_replacement( population_size: usize, subsample_ratio: f64, @@ -606,7 +611,10 @@ impl, Y: Array1> XGRegres ) -> Vec { let mut indices: Vec = (0..population_size).collect(); indices.shuffle(rng); - indices.truncate((population_size as f64 * subsample_ratio) as usize); + // `population_size * subsample_ratio` truncates to 0 for a small population, e.g. 3 rows + // at a ratio of 0.3. Keep one row in that case, as scikit-learn does for its own + // `subsample` parameter (#444). + indices.truncate(((population_size as f64 * subsample_ratio) as usize).max(1)); indices } } @@ -762,6 +770,95 @@ mod tests { assert_eq!(predictions.unwrap().len(), 2); } + /// `subsample` < 1.0 must not panic when the sample size truncates to zero rows. + #[test] + fn test_subsample_smaller_than_one_sample_does_not_panic() { + let x_vec = vec![vec![1.0, 1.0], vec![2.0, 1.0], vec![1.0, 2.0]]; + let x = DenseMatrix::from_2d_vec(&x_vec).unwrap(); + let y = vec![5.0, 7.0, 8.0]; + + // 3 samples * 0.3 truncates to 0 rows, so the tree is fit on an empty index set. + let params = XGRegressorParameters::default() + .with_n_estimators(5) + .with_max_depth(3) + .with_subsample(0.3); + + let model = XGRegressor::fit(&x, &y, params); + assert!(model.is_ok(), "Fit failed: {:?}", model.err()); + + let predictions: Vec = model.unwrap().predict(&x).unwrap(); + assert_eq!(predictions.len(), 3); + assert!(predictions.iter().all(|p| p.is_finite())); + } + + /// A single-row dataset with any `subsample` < 1.0 also truncates to zero rows. + #[test] + fn test_subsample_on_single_row_does_not_panic() { + let x_vec = vec![vec![1.0, 1.0]]; + let x = DenseMatrix::from_2d_vec(&x_vec).unwrap(); + let y = vec![5.0]; + + let params = XGRegressorParameters::default() + .with_n_estimators(5) + .with_max_depth(3) + .with_subsample(0.9); + + let model = XGRegressor::fit(&x, &y, params); + assert!(model.is_ok(), "Fit failed: {:?}", model.err()); + + let predictions: Vec = model.unwrap().predict(&x).unwrap(); + assert_eq!(predictions.len(), 1); + assert!(predictions[0].is_finite()); + } + #[test] + fn test_sample_without_replacement_clamps_to_one_row() { + // (population, ratio): each pair gives `floor(population * ratio) == 0`. + let cases = [(3, 0.3), (2, 0.4), (1, 0.9), (10, 0.05), (5, 1e-12)]; + + for (population, ratio) in cases { + let mut rng = get_rng_impl(Some(42)); + let sample = + XGRegressor::, Vec>::sample_without_replacement( + population, ratio, &mut rng, + ); + + assert_eq!( + sample.len(), + 1, + "population {population} at ratio {ratio} gave {sample:?}" + ); + assert!(sample[0] < population); + } + } + + /// Regression guard: the clamp must not change a sample size that is already one or more. + /// A ratio of 0.8 must still take 80% of the rows, not all of them. + #[test] + fn test_sample_without_replacement_keeps_the_ratio_above_one_row() { + // (population, ratio, expected sample size) + let cases = [(2, 0.5, 1), (2, 1.0, 2), (10, 0.8, 8), (100, 0.8, 80)]; + + for (population, ratio, expected) in cases { + let mut rng = get_rng_impl(Some(42)); + let mut sample = + XGRegressor::, Vec>::sample_without_replacement( + population, ratio, &mut rng, + ); + + assert_eq!( + sample.len(), + expected, + "population {population} at ratio {ratio} gave {sample:?}" + ); + assert!(sample.iter().all(|&i| i < population)); + + // The indices must stay unique, i.e. the sample is without replacement. + sample.sort_unstable(); + sample.dedup(); + assert_eq!(sample.len(), expected); + } + } + /// A "smoke test" to ensure the main XGRegressor can fit and predict on multidimensional data. #[test] fn test_xgregressor_fit_predict_multidimensional() {