Skip to content

Commit 1151d89

Browse files
authored
Merge pull request #964 from mlr-org/integer_overflow
fix: integer overflow in remove constants
2 parents 7061a32 + f87b47e commit 1151d89

File tree

3 files changed

+18
-1
lines changed

3 files changed

+18
-1
lines changed

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* Added support for internal validation tasks to `PipeOpFeatureUnion`.
77
* feat: `PipeOpLearnerCV` can reuse the cross-validation models during prediction by averaging their outputs (`resampling.predict_method = "cv_ensemble"`).
88
* feat: `PipeOpRegrAvg` gets new `se_aggr` and `se_aggr_rho` hyperparameters and now allows various forms of SE aggregation.
9+
* Fix: `PipeOpRemoveConstants` now avoids integer overflow when evaluating relative tolerances for near-`integer.max` data.
910
* Compatibility with new testthat version 3.3.0
1011

1112
# mlr3pipelines 0.9.0

R/PipeOpRemoveConstants.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ is_constant_enough = function(x, ratio, rel_tol, abs_tol, na_ignore) {
109109

110110
# now consider finite values: sort them and see if items that are
111111
# 'required_size - 1' steps away from each other differ by at most 'abs_tol' or 'rel_tol'
112-
x = sort(x[is.finite(x)])
112+
x = as.numeric(sort(x[is.finite(x)]))
113113
if (length(x) < required_size) {
114114
return(FALSE)
115115
}

tests/testthat/test_pipeop_removeconstants.R

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,19 @@ test_that("PipeOpRemoveConstants removes expected cols", {
7070
test_dropping(minus.iris , minus.iris, list())
7171

7272
})
73+
74+
test_that("PipeOpRemoveConstants handles integer overflow constants", {
75+
n = 5L
76+
big = .Machine$integer.max
77+
dt = data.table(
78+
const = c(rep.int(big, n - 1L), big - 1L),
79+
vary = c(big, big - 10L, big - 20L, big - 30L, big - 40L),
80+
target = factor(c("a", "a", "b", "b", "b"))
81+
)
82+
83+
task = TaskClassif$new("overflow", dt, target = "target")
84+
po = PipeOpRemoveConstants$new(param_vals = list(abs_tol = 0))
85+
86+
result = po$train(list(task))[[1]]
87+
expect_false("const" %in% result$feature_names)
88+
})

0 commit comments

Comments
 (0)