Skip to content

Commit

Permalink
Fix "vce" bug with length 1 "variables" argument (leeper#113)
Browse files Browse the repository at this point in the history
* Fix vce = "simulation" when variables has length of 1
* Fix vce = "bootstrap" when variables has length of 1
* Add name to DESCRIPTION
* Update NEWS.md
  • Loading branch information
jacob-long authored and leeper committed Dec 23, 2019
1 parent b9ad25f commit f4054c5
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 4 deletions.
5 changes: 2 additions & 3 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

* Setup a `cplot.default()` method and modified documentation of `cplot()`, `image()`, and `persp()` methods slightly. (#84, h/t Luke Sonnet)
* Improve the documentation the behavior of `cplot()` for generalized linear models, which can generate unexpected confidence intervals (albeit ones consistent with base R's behavior). (#92)
* Models fit using the `lme4` package can now have variance estimation via
bootstrap and simulation (#105).
* Fix bug that caused spurious `NA`s and errors in `margins()` when `vce` was `"bootstrap"` or `"simulation"` and `variables` had a length of 1. (#112)
* Models fit using the `lme4` package can now have variance estimation via bootstrap and simulation (#105).

# margins 0.3.24

Expand Down Expand Up @@ -44,7 +44,6 @@ bootstrap and simulation (#105).
* Updated examples in `README.Rmd`. (#83)

# margins 0.3.16
>>>>>>> master

* Fixed a bug in `cplot()` when `xvar` was of class "ordered". (#77, h/t Francisco Llaneras)
* Fixed a bug in `plot.margins()` when `at` contained only one variable. (#78, h/t @cyberbryce)
Expand Down
15 changes: 14 additions & 1 deletion R/get_effect_variances.R
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,13 @@ function(data,
}
return(means)
})
# When length(variables) == 1, effectmat is a vector
if (!is.matrix(effectmat)) {
# Coerce to 1 row matrix
effectmat <- matrix(effectmat, nrow = 1)
# Rownames are lost in these cases
rownames(effectmat) <- paste0("dydx_", variables)
}
# calculate the variance of the simulated AMEs
vc <- var(t(effectmat))
variances <- diag(vc)
Expand Down Expand Up @@ -164,7 +171,13 @@ function(data,
means
}
# bootstrap the data and take the variance of bootstrapped AMEs
vc <- var(t(replicate(iterations, bootfun())))
vc <- if (length(variables) > 1) {
var(t(replicate(iterations, bootfun())))
} else { # Take the variance of the vector
# Need to coerce to 1 x 1 matrix with appropriate dimnames
matrix(var(replicate(iterations, bootfun())), nrow = 1L,
dimnames = list(nms <- paste0("dydx_", variables), nms))
}
variances <- diag(vc)
jacobian <- NULL
}
Expand Down
14 changes: 14 additions & 0 deletions tests/testthat/tests-edge-cases.R
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,17 @@ test_that("margins() errors correctly when there are no RHS variables", {
expect_error(marginal_effects(x))
expect_error(margins(x))
})

test_that("vce = 'bootstrap' works with one variable as variables argument", {
x <- lm(mpg ~ wt + hp * cyl, data = mtcars)
suppressWarnings(m <- margins(x, vce = "bootstrap", variables = "hp"))
expect_true(inherits(m, "data.frame"))
expect_true(nrow(summary(m)) == 1L)
})

test_that("vce = 'simulation' works with one variable as variables argument", {
x <- lm(mpg ~ wt + hp * cyl, data = mtcars)
suppressWarnings(m <- margins(x, vce = "simulation", variables = "hp"))
expect_true(inherits(m, "data.frame"))
expect_true(nrow(summary(m)) == 1L)
})

0 comments on commit f4054c5

Please sign in to comment.