Skip to content

Commit f13e692

Browse files
authored
Merge pull request #542 from aloctavodia/susi
suggest_size(): Return nterms_max instead of NA when no model satisfies the criterion
2 parents 13d6b93 + ca5c059 commit f13e692

2 files changed

Lines changed: 26 additions & 24 deletions

File tree

R/methods.R

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1820,16 +1820,13 @@ suggest_size.vsel <- function(
18201820

18211821
if (nrow(res) == 0) {
18221822
## no submodel satisfying the criterion found
1823-
if (object$nterms_max == object$nterms_all) {
1824-
suggested_size <- object$nterms_max
1825-
} else {
1826-
suggested_size <- NA
1827-
if (warnings) {
1828-
warning("Could not suggest submodel size. Investigate plot.vsel() to ",
1829-
"identify if the search was terminated too early. If this is ",
1830-
"the case, run variable selection with larger value for ",
1831-
"`nterms_max`.")
1832-
}
1823+
suggested_size <- object$nterms_max
1824+
if (object$nterms_max != object$nterms_all && warnings) {
1825+
warning("Could not suggest submodel size within the searched range. ",
1826+
"Investigate plot.vsel() to identify if the search was ",
1827+
"terminated too early or the criterion is too strict.",
1828+
"If this is the case, run variable selection with ",
1829+
"larger value for `nterms_max` or use less strict criterion.")
18331830
}
18341831
} else {
18351832
suggested_size <- min(res)

tests/testthat/test_methods_vsel.R

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -580,28 +580,33 @@ test_that("`stat` works", {
580580
} else {
581581
suggsize_seed <- NULL
582582
}
583-
# Warnings are suppressed, but a suggested size of `NA` (because of a
584-
# search which was terminated too early) is tested below:
583+
# Warnings are suppressed, but a suggested size of nterms_max (because of
584+
# a search which was terminated too early) is tested below:
585585
suggsize <- suppressWarnings(
586586
suggest_size(vss[[tstsetup_vs]], stat = stat_crr, seed = suggsize_seed)
587587
)
588588
expect_length(suggsize, 1)
589-
if (!is.na(suggsize)) {
590-
expect_true(is.vector(suggsize, "numeric"),
591-
info = paste(tstsetup, stat_crr, sep = "__"))
592-
expect_true(suggsize >= 0, info = paste(tstsetup, stat_crr, sep = "__"))
593-
} else {
594-
expect_identical(suggsize, NA,
595-
info = paste(tstsetup, stat_crr, sep = "__"))
596-
expect_true(
597-
vss[[tstsetup_vs]]$nterms_max < vss[[tstsetup_vs]]$nterms_all,
598-
info = paste(tstsetup, stat_crr, sep = "__")
599-
)
600-
}
589+
expect_true(is.vector(suggsize, "numeric"),
590+
info = paste(tstsetup, stat_crr, sep = "__"))
591+
expect_true(suggsize >= 0, info = paste(tstsetup, stat_crr, sep = "__"))
592+
# If the search was terminated early and no submodel satisfies the
593+
# criterion, suggest_size() should return nterms_max:
594+
expect_true(suggsize <= vss[[tstsetup_vs]]$nterms_max,
595+
info = paste(tstsetup, stat_crr, sep = "__"))
601596
}
602597
}
603598
})
604599

600+
test_that("returns nterms_max when no criterion is met", {
601+
skip_if_not(run_vs)
602+
tstsetup <- head(names(vss), 1)
603+
suggsize <- suppressWarnings(
604+
suggest_size(vss[[tstsetup]], stat = "elpd", pct = -1.0)
605+
)
606+
expect_identical(suggsize, vss[[tstsetup]]$nterms_max,
607+
info = tstsetup)
608+
})
609+
605610
# ranking() ---------------------------------------------------------------
606611

607612
context("ranking()")

0 commit comments

Comments
 (0)