From 5375e3b81104796f7b0ad05558cdb64501aa0f6c Mon Sep 17 00:00:00 2001 From: Aki Vehtari Date: Tue, 13 Oct 2026 19:43:40 +0300 Subject: [PATCH 1/3] fix: clamp Pareto-smoothed Pathfinder init weights at 0 --- R/args.R | 3 ++ tests/testthat/test-fit-init.R | 56 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/R/args.R b/R/args.R index 2e1efd205..180dc598f 100644 --- a/R/args.R +++ b/R/args.R @@ -1381,6 +1381,9 @@ process_init_approx <- function(init, num_procs, model_variables = NULL, } else { weights <- posterior::pareto_smooth( exp(log_weights - max(log_weights)), tail = "right", r_eff=1, return_k=FALSE) + # posterior <= 1.7.1 can return weights of about -2e-16 when many weights + # underflow to 0 + weights <- pmax(weights, 0) } if (num_candidates < nrow(draws_df)) { # Collapse duplicate candidates to one row, summing their weights so a vector diff --git a/tests/testthat/test-fit-init.R b/tests/testthat/test-fit-init.R index da602862f..b21d65c67 100644 --- a/tests/testthat/test-fit-init.R +++ b/tests/testthat/test-fit-init.R @@ -147,6 +147,62 @@ test_that("Pathfinder fit initializations use the intended weights", { expect_equal(pareto_smooth_calls, 2L) }) +test_that("Pathfinder init weights are not negative after Pareto smoothing", { + # Most log weights are far below the maximum, so exp() underflows to 0 for + # them. posterior::pareto_smooth() can then return weights of about -2e-16 + # (posterior <= 1.7.1), which resample_draws() rejects. + lp <- c(-1000 - seq_len(350), -seq(0, 60, length.out = 50)) + pathfinder_fit <- function(draws) { + structure( + list( + draws = function(format) draws, + metadata = function() list( + num_paths = 10, + psis_resample = FALSE, + calculate_lp = TRUE + ), + return_codes = function() 0 + ), + class = "CmdStanPathfinder" + ) + } + local_mocked_bindings(process_init = function(init, ...) init) + + draws <- posterior::as_draws_df(data.frame( + theta = seq_along(lp), + lp__ = lp, + lp_approx__ = 0 + )) + inits <- NULL + expect_no_error( + inits <- suppressMessages( + process_init.CmdStanPathfinder(pathfinder_fit(draws), 4) + ) + ) + expect_equal(posterior::ndraws(inits), 4) + + # The same with a smoother that returns negative weights, so that the test + # does not depend on the installed posterior version. The first three draws + # are duplicates, so their weights are summed. + draws_dup <- posterior::as_draws_df(data.frame( + theta = c(1, 1, 1, seq_along(lp)[-(1:3)]), + lp__ = c(rep(lp[1], 3), lp[-(1:3)]), + lp_approx__ = 0 + )) + weights_seen <- NULL + local_mocked_bindings( + pareto_smooth = function(x, ...) x - 1e-16, + resample_draws = function(x, ndraws, weights = NULL, ...) { + weights_seen <<- weights + x[seq_len(ndraws), , drop = FALSE] + }, + .package = "posterior" + ) + process_init.CmdStanPathfinder(pathfinder_fit(draws_dup), 4) + expect_length(weights_seen, length(lp) - 2) + expect_true(all(weights_seen >= 0)) +}) + test_that("Pathfinder init candidates are distinct target parameter vectors", { draws <- posterior::as_draws_df(data.frame( theta = c(1, 1, 2, 3), From 864cdd229e177d9118665949bc2f3de2d5697f1e Mon Sep 17 00:00:00 2001 From: jgabry Date: Tue, 13 Oct 2026 11:15:41 -0600 Subject: [PATCH 2/3] Test the Pathfinder weight clamp with only the smoother mocked --- tests/testthat/test-fit-init.R | 64 +++++++++++----------------------- 1 file changed, 20 insertions(+), 44 deletions(-) diff --git a/tests/testthat/test-fit-init.R b/tests/testthat/test-fit-init.R index 7aa02b7a0..20d41d75b 100644 --- a/tests/testthat/test-fit-init.R +++ b/tests/testthat/test-fit-init.R @@ -152,57 +152,33 @@ test_that("Pathfinder fit initializations use the intended weights", { test_that("Pathfinder init weights are not negative after Pareto smoothing", { # Most log weights are far below the maximum, so exp() underflows to 0 for # them. posterior::pareto_smooth() can then return weights of about -2e-16 - # (posterior <= 1.7.1), which resample_draws() rejects. + # (posterior <= 1.7.1), which resample_draws() rejects. The mock gives those + # weights whatever the installed posterior version. lp <- c(-1000 - seq_len(350), -seq(0, 60, length.out = 50)) - pathfinder_fit <- function(draws) { - structure( - list( - draws = function(format) draws, - metadata = function() list( - num_paths = 10, - psis_resample = FALSE, - calculate_lp = TRUE - ), - return_codes = function() 0 + pathfinder_fit <- structure( + list( + draws = function(format) posterior::as_draws_df(data.frame( + theta = seq_along(lp), + lp__ = lp, + lp_approx__ = 0 + )), + metadata = function() list( + num_paths = 10, + psis_resample = FALSE, + calculate_lp = TRUE ), - class = "CmdStanPathfinder" - ) - } - local_mocked_bindings(process_init = function(init, ...) init) - - draws <- posterior::as_draws_df(data.frame( - theta = seq_along(lp), - lp__ = lp, - lp_approx__ = 0 - )) - inits <- NULL - expect_no_error( - inits <- suppressMessages( - process_init.CmdStanPathfinder(pathfinder_fit(draws), 4) - ) + return_codes = function() 0 + ), + class = "CmdStanPathfinder" ) - expect_equal(posterior::ndraws(inits), 4) - - # The same with a smoother that returns negative weights, so that the test - # does not depend on the installed posterior version. The first three draws - # are duplicates, so their weights are summed. - draws_dup <- posterior::as_draws_df(data.frame( - theta = c(1, 1, 1, seq_along(lp)[-(1:3)]), - lp__ = c(rep(lp[1], 3), lp[-(1:3)]), - lp_approx__ = 0 - )) - weights_seen <- NULL local_mocked_bindings( pareto_smooth = function(x, ...) x - 1e-16, - resample_draws = function(x, ndraws, weights = NULL, ...) { - weights_seen <<- weights - x[seq_len(ndraws), , drop = FALSE] - }, .package = "posterior" ) - process_init.CmdStanPathfinder(pathfinder_fit(draws_dup), 4) - expect_length(weights_seen, length(lp) - 2) - expect_true(all(weights_seen >= 0)) + local_mocked_bindings(process_init = function(init, ...) init) + + inits <- process_init.CmdStanPathfinder(pathfinder_fit, 4) + expect_equal(posterior::ndraws(inits), 4) }) test_that("Pathfinder init candidates are distinct target parameter vectors", { From 65618cca27d19f11619feb0950dd445f6909182a Mon Sep 17 00:00:00 2001 From: jgabry Date: Tue, 13 Oct 2026 11:21:08 -0600 Subject: [PATCH 3/3] Say when to remove the Pathfinder weight clamp and its test --- R/args.R | 3 ++- tests/testthat/test-fit-init.R | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/R/args.R b/R/args.R index 4e413e802..708889cd7 100644 --- a/R/args.R +++ b/R/args.R @@ -1337,7 +1337,8 @@ process_init_approx <- function(init, num_procs, model_variables = NULL, return_k = FALSE, verbose = FALSE ) # posterior <= 1.7.1 can return weights of about -2e-16 when many weights - # underflow to 0 + # underflow to 0. Remove this once the minimum posterior version includes + # stan-dev/posterior#468. weights <- pmax(weights, 0) } if (num_candidates < nrow(draws_df)) { diff --git a/tests/testthat/test-fit-init.R b/tests/testthat/test-fit-init.R index 20d41d75b..e13e66727 100644 --- a/tests/testthat/test-fit-init.R +++ b/tests/testthat/test-fit-init.R @@ -153,7 +153,9 @@ test_that("Pathfinder init weights are not negative after Pareto smoothing", { # Most log weights are far below the maximum, so exp() underflows to 0 for # them. posterior::pareto_smooth() can then return weights of about -2e-16 # (posterior <= 1.7.1), which resample_draws() rejects. The mock gives those - # weights whatever the installed posterior version. + # weights whatever the installed posterior version. Remove this test with + # the clamp once the minimum posterior version includes + # stan-dev/posterior#468. lp <- c(-1000 - seq_len(350), -seq(0, 60, length.out = 50)) pathfinder_fit <- structure( list(