Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Over Conflicted? #110

Closed
emstruong opened this issue Apr 29, 2024 · 11 comments
Closed

Over Conflicted? #110

emstruong opened this issue Apr 29, 2024 · 11 comments

Comments

@emstruong
Copy link

Hello,

This is related to this issue--is it possible that conflicted might be over-alerting?

I'm not entirely sure I understand what's going on--and I might be butchering my comp-sci--but it seems like it's detecting conflicts between the namespace of tidyverse and data.table, which is caused by some downstream effect of whatever code dynamite is running. Is this possibly a false conflict or is it an actual cause for concern?

@hadley
Copy link
Member

hadley commented Apr 30, 2024

Anything is possible, but the place to start would be with a reprex.

@emstruong
Copy link
Author

Anything is possible, but the place to start would be with a reprex.

So this is what seems to be happening

library(conflicted)
library(data.table)
library(tidyverse)
library(dynamite)
fit <- dynamite(
  dformula = obs(y ~ -1 + varying(~x), family = "gaussian") +
    lags(type = "varying") +
    splines(df = 20),
  gaussian_example,
  "time",
  "id",
  chains = 1,
  refresh = 0
)
#> Error:
#> ! [conflicted] between found in 2 packages.
#> Either pick the one you want with `::`:
#> • dplyr::between
#> • data.table::between
#> Or declare a preference with `conflicts_prefer()`:
#> • `conflicts_prefer(dplyr::between)`
#> • `conflicts_prefer(data.table::between)`

Created on 2024-04-14 with reprex v2.1.0

And my understanding is that the situation is unexpected to the authors of dynamite because the code-base for dynamite itself doesn't use a between() in its codebase.

My (their) guess is that dynamite is (probably) stringing up Stan code that is executed by some other package down the line, which does use between() -- RStan. However, I thought that RStan shouldn't be affected by anyone calling library(tidyverse) or library(data.table), because it'll know what between() function it should use based on the Imports field of its DESCRIPTION file...

@hadley
Copy link
Member

hadley commented Apr 30, 2024

That suggests that dynamite is evaluating the generated code in the global environment. If you look at the traceback:

 ▆
  1. ├─dynamite::dynamite(...)
  2. │ └─dynamite:::dynamite_stan(...)
  3. │   ├─dynamite:::ifelse_(...)
  4. │   └─dynamite:::dynamite_model(...)
  5. │     ├─base::with(...)
  6. │     └─base::with.default(...)
  7. │       └─base::eval(substitute(expr), data, enclos = parent.frame())
  8. │         └─base::eval(substitute(expr), data, enclos = parent.frame())
  9. │           └─rstan::stan_model(model_code = model_code)
 10. │             └─utils::apropos(model_re, mode = "S4", ignore.case = FALSE)
 11. │               └─base::vapply(li, exists, NA, where = i, mode = mode, inherits = FALSE)
 12. │                 └─base (local) FUN(X[[i]], ...)
 13. └─conflicted (local) `<fn>`()
 14.   └─cli::cli_abort(...)
 15.     └─rlang::abort(...)

It's likely to be something to do with the way they're using with. (But it's not clear to me where the between() is coming in)

@emstruong
Copy link
Author

That suggests that dynamite is evaluating the generated code in the global environment. If you look at the traceback:

 ▆
  1. ├─dynamite::dynamite(...)
  2. │ └─dynamite:::dynamite_stan(...)
  3. │   ├─dynamite:::ifelse_(...)
  4. │   └─dynamite:::dynamite_model(...)
  5. │     ├─base::with(...)
  6. │     └─base::with.default(...)
  7. │       └─base::eval(substitute(expr), data, enclos = parent.frame())
  8. │         └─base::eval(substitute(expr), data, enclos = parent.frame())
  9. │           └─rstan::stan_model(model_code = model_code)
 10. │             └─utils::apropos(model_re, mode = "S4", ignore.case = FALSE)
 11. │               └─base::vapply(li, exists, NA, where = i, mode = mode, inherits = FALSE)
 12. │                 └─base (local) FUN(X[[i]], ...)
 13. └─conflicted (local) `<fn>`()
 14.   └─cli::cli_abort(...)
 15.     └─rlang::abort(...)

It's likely to be something to do with the way they're using with. (But it's not clear to me where the between() is coming in)

Sorry for the late reply. Perhaps this is a separate issue, but the dynamite developers also make the point that conflicted is finding conflicts in unexpected places. For example

library(conflicted)
library(dplyr)
apropos("f", mode = "S4")
#> Error:
#> ! [conflicted] filter found in 2 packages.
#> Either pick the one you want with `::`:
#> • dplyr::filter
#> • stats::filter
#> Or declare a preference with `conflicts_prefer()`:
#> • `conflicts_prefer(dplyr::filter)`
#> • `conflicts_prefer(stats::filter)`

Created on 2024-07-26 with reprex v2.1.0

Is it the same issue?

@hadley
Copy link
Member

hadley commented Jul 26, 2024

The problem is that when you set a mode in apropos() it has to evaluate every binding, and that triggers conflicted. You could maybe argue that this is a bug in apropos() (maybe it should ignore active bindings?) but I think it's just an unfortunate interaction.

@emstruong
Copy link
Author

Sorry again for the late reply.

If I understand what's happening, so then would the solution to either be to change conflicted so that it doesn't get triggered by apropos() specifically or to change apropos() to skip .conflicts() when it sees it in sp <- search()?

Perhaps unsurprisingly, it doesn't seem like base R has a Github repo where I could post this kind of issue and I don't have the bandwidth to dig in the conflicted codebase. If I were to try to email the R utils maintainer ([email protected]?) or r-devel (here?), I guess they might want to make .conflicts into a special keyword that no other package or function could use in this context?

Should I close the issue unless someone would like to take this up?

While it is an unfortunate interaction, it seems like a complicated problem. It seems like the only workaround would be to essentially not use library(conflicted) at all when using other functions that involve apropos(), unless one is 'lucky' about what packages they use. This seems tough because RStan uses apropos() and if you're using Bayesian methods you're probably already using a bunch of other packages.

@hadley
Copy link
Member

hadley commented Oct 16, 2024

If you're hitting this problem because RStan is using apropros(), I think you should file the issue there — IMO apropros() is something that should be used directly by the user, not wrapped into another function used by a package.

@emstruong
Copy link
Author

If you're hitting this problem because RStan is using apropros(), I think you should file the issue there — IMO apropros() is something that should be used directly by the user, not wrapped into another function used by a package.

That may be it

@emstruong
Copy link
Author

So I don't know if it's valid to re-open the issue, but it's worth mentioning that the same kind of issue happens in brms. I'm not sure about the details because it seems like brms uses a custom do.call function called do_call. I'm going to guess that this kind of issue arises anytime a developer tries to grab the entire environment and do something with it.

I'll link the discussion with the brms repo.

library(brms)
#> Loading required package: Rcpp
#> Loading 'brms' package (version 2.20.4). Useful instructions
#> can be found by typing help('brms'). A more detailed introduction
#> to the package is available through vignette('brms_overview').
#> 
#> Attaching package: 'brms'
#> The following object is masked from 'package:stats':
#> 
#>     ar
library(conflicted)
library(reprex)
# From Example Documentation
prior1 <- prior(normal(0,10), class = b) +
  prior(cauchy(0,2), class = sd)
fit1 <- brms::brm(count ~ zAge + zBase * Trt + (1|patient),
            data = epilepsy, family = poisson(), prior = prior1)
#> Compiling Stan program...
#> Error:
#> ! [conflicted] ar found in 2 packages.
#> Either pick the one you want with `::`:
#> • brms::ar
#> • stats::ar
#> Or declare a preference with `conflicts_prefer()`:
#> • `conflicts_prefer(brms::ar)`
#> • `conflicts_prefer(stats::ar)`

Created on 2024-10-23 with reprex v2.1.1

@emstruong emstruong reopened this Oct 23, 2024
@emstruong
Copy link
Author

I guess I'm also wondering--if I had run conflicts_prefer(brms::ar) and some other package uses stats::ar, will the other package's behavior suddenly change to use brms::ar instead of stats::ar?

@emstruong
Copy link
Author

Never mind, it's because of apropos in Rstan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants