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

mirai don't start with custom .libPaths nor options #122

Closed
gogonzo opened this issue Jul 12, 2024 · 3 comments
Closed

mirai don't start with custom .libPaths nor options #122

gogonzo opened this issue Jul 12, 2024 · 3 comments

Comments

@gogonzo
Copy link

gogonzo commented Jul 12, 2024

I don't know whether this is really a bug, or maybe you envision mirai's task to start on vanilla R session not inheriting any information from parent session (where it is invoked from). In my case I have an .Rprofile file and .Renviron file in a "project" directory and my "parent session" includes custom options and custom .libPaths(). mirai seem to start an R session in system default directory.

Let's imagine you have some options() and .libPaths() changed through .Rprofile

failing solution

.libPaths("~/new_dir")
options(repos = c(myrepo = "myrepo.mydomain"))
run_fun <- function() {
  list(
    repos = options()$repos,
    libpaths = .libPaths()
  )    
}

task1 <- mirai::mirai(run(), run = run_fun)
identical(task1$data, run_fun())
# FALSE

working solution

I'm forced to set the process in a way where I set options and .libPaths through the args.

.libPaths("~/new_dir")
options(repos = c(myrepo = "myrepo.mydomain"))
run_fun_with_opts <- function(opts, libpaths) {
  options(opts)
  .libPaths(libpaths)
  list(
    repos = options()$repos,
    libpaths = .libPaths()
  )
}
task2 <- mirai::mirai(run(opts, libpaths), run = run_fun_with_opts, opts = options(), libpaths = .libPaths())
identical(task2$data, run_fun())
# TRUE

I think it makes sense to start an R process by default from the same directory as working directory (to include .Rprofile or .Renviron). Only options or system vars set outside of the .Rprofile and .Renviron should be handled by the user. What is your opinion about this?

@shikokuchuo
Copy link
Owner

shikokuchuo commented Jul 12, 2024

It is by design that global options and the state of the current session (e.g. working directory) do not affect mirai evaluation.

EDIT: @gogonzo Just to expand a little on the reasoning behind this design choice - (i) it is never clear what is relevant to be 'exported' for example the working directory is important in your case but may not be obvious / necessary for others (ii) a 'maximalist' approach would be to try and cover 'everything', but that runs into the first issue that expectation may still not be matched, and also sometimes - for example a mirai may source a script in another directory - what should the working directory be in that case - the intent would always have to be a guess. So mirai takes a minimalist approach, and this (i) allows consistent and predictable behaviour, (ii) incurs the lowest overhead by not doing all this by default, and (iii) still allows the flexibility for you to do the setup exactly as you wish.

The daemon will use the default .Rprofile etc. on the machine it is running on.

Note that if you set .libPaths to .libPaths("~/new_dir") first then possibly you can't load mirai in the first place unless it is also in that library.

If you want to change the options etc. and persist them, then I suggest you make a call to everywhere() instead:

mirai::everywhere(run(opts, libpaths), .args = list(run = run_fun_with_opts, opts = options(), libpaths = .libPaths()))

@shikokuchuo
Copy link
Owner

Note that everywhere() is precisely meant for these type of setup tasks.

Doing it this way is actually more efficient than trying to deparse a hypothetical argument to daemons() and pass it via the command line. So at the moment there is no automatic detection of these variables going on, but even if there were it would be optimal to send them via an everywhere() call.

Hope this explanation helps. If you have any suggestions do let me know.

@gogonzo
Copy link
Author

gogonzo commented Jul 13, 2024

(i) it is never clear what is relevant to be 'exported' for example the working directory is important in your case but may not be obvious / necessary for others (ii) a 'maximalist' approach would be to try and cover 'everything', but that runs into the first issue that expectation may still not be matched, and also sometimes - for example a mirai may source a script in another directory - what should the working directory be in that case - the intent would always have to be a guess. So mirai takes a minimalist approach, and this (i) allows consistent and predictable behaviour

Thanks @shikokuchuo I see, good point 👍 This is non-trivial issue we face. We just need a default setting from .Rprofile
and .Renviron but we can't really guarantee that getwd() is really the directory in which R started. Possibly .Rprofile could use setwd() which means that collecting any informations doesn't guarantee reproducibility in our case.

Note that everywhere() is precisely meant for these type of setup tasks.

Thanks for a suggestion. We can't really set default vars through everywhere() as our framework executes mirai possibly after and possibly before users custom execution. It means that we could overwrite one user's everything() setup, so we need to be careful. In our case it would be safer to just start independent mirai::mirai and let user set anything without us affecting users process and user affecting our process.


I think all makes sense. You presented well mirai's approach and it is clear no how we should utilise your package.
Thanks

@gogonzo gogonzo closed this as completed Jul 13, 2024
gogonzo added a commit to insightsengineering/teal that referenced this issue Sep 23, 2024
#1276

@pawelru @m7pr 
This is a `mirai` alternative. Package seems to address the biggest
issues reported during a research:
- `mirai` has a native support of `ExtendedTask`
- `mirai` is not being killed when `runApp` is executed (like `callr`
does)
- `mirai` by default opens a deamon in parallel R session without a need
to handle the `future::plan`.
- `mirai` has only one dependency in the whole dependency tree.

#### Disadvantages so far: 
- ~~we need to pass and set `options`, system vars, working directory
and `.libPaths` shikokuchuo/mirai#122

#### How does it work:
- lockfile creation is invoked in `init` before application starts. This
prevents to start the process each time when a new shiny session starts.
Process is invoked as a promise and eventually `teal_app.lock` will be
created
- When shiny session starts `download lockfile` button is hidden by
default. If promise is eventually resolved and lockfile is created then
download button is shown.
- alternatively, app developer can pre-compute lockfile and provide its
path in `teal.renv.lockfile` option. In such case `renv::snapshot` will
be skipped and user lockfile will be used in an app.

#### Logs and notifications
Logs are printed for app developer while notifications are presented to
the app user:
1. When app uses precomputed file:
- log in init: `Lockfile set using option "teal.renv.lockfile" -
skipping automatic creation.`
- no notification to the app user.
2. When app automatically determines snapshot:
- log in init: `Lockfile creation started based on { getwd() }.`
- log If lockfile created: `Lockfile {path} containing { n-pkgs }
packages created{ with errors or warnings }.`
- notification if lockfile created: `Lockfile available to download`
- log if lockfile not created: `Lockfile creation failed.` 
- notification if lockfile not created: `Lockfile creation failed.`

---------

Signed-off-by: Marcin <[email protected]>
Signed-off-by: Pawel Rucki <[email protected]>
Co-authored-by: m7pr <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Marcin <[email protected]>
Co-authored-by: Pawel Rucki <[email protected]>
Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Aleksander Chlebowski <[email protected]>
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