tolerance for upcoming `marginaleffects`

[vincentarelbundock]

You've see the discussion about incorrect standard errors here: https://github.com/vincentarelbundock/marginaleffects/issues/684

I consider this a critical issue because it affects numerical accuracy, and I'd like to release a quick fix.

TBC, the issue does not appear to affect etwfe much, if at all. Unfortunately, changing the step size in my finite difference delta method code does change some digits and it will make your tests fail on CRAN. (Having to update my packages for tiny upstream changes like this is why I don't use CRAN for CI anymore.)

This PR adds tolerance=5e-4 to some of your expect_equivalent() calls.

I know you just released a version because of me and that this is annoying. I'll do my best to check rev-deps and go slower in the future.

[grantmcdermott]

Thanks bud, much appreciated. I’ve been wanting to fix some small things in the vignette anyway and the speed gains from the new marginaleffects release are reason enough to release a patch update.

Having to update my packages for tiny upstream changes like this is why I don't use CRAN for CI anymore.

Curious about this. Do you mean that you hide your test suite from CRAN?

[vincentarelbundock]

Cool cool.

Curious about this. Do you mean that you hide your test suite from CRAN?

I just skip them entirely in tests/tinytest.R using an environment variable:

if (requireNamespace("tinytest", quietly = TRUE) &&
    isTRUE(Sys.getenv("R_NOT_CRAN") == "true") &&
    isTRUE(Sys.info()["sysname"] != "Windows")) {
    tinytest::test_package("marginaleffects")
}

This is obviously bad practice, but modelsummary and marginaleffects are weird cases: they interop with dozens of Suggests packages and I was constantly having to make new releases for tiny changes in tolerance.

[vincentarelbundock]

One note on the PR: you'll have to remove the Remotes: line I added before releasing to CRAN.

[vincentarelbundock]

BTW, on speed @etiennebacher just made another great PR: https://github.com/vincentarelbundock/marginaleffects/pull/690

[grantmcdermott]

BTW, on speed @etiennebacher just made another great PR: https://github.com/vincentarelbundock/marginaleffects/pull/690

Crushing it. I know you want to get this update out quickly for the SE step tolerance. But please do highlight the extent of these speed gains when you release. (The current NEWS item buries the lede here a bit too much IMO… you’re getting 4-5x improvements for large models AFAICT and maybe even more with this latest PR. That’s well worth saying explicitly.)