should install_tensorflow delegate to py_install?

[kevinushey]

The main motivations:

1. py_install() will automatically infer the appropriate environment type if e.g. RETICULATE_PYTHON_ENV is set; right now in tensorflow it seems "auto" means "virtualenv" (except on Windows) but one might have an active conda environment they want to use instead.

2. reticulate understands how to use environments both by name and path; it seems like the tensorflow shims only understand environments by name.

If required, a parameter for destroying and re-creating the environment could be kept (ie as a guard against buggy TensorFlow updates).

[jjallaire]

Should already be done via #333

[kevinushey]

IIUC tensorflow currently skips py_install() and tries to use the virtualenv_*() and conda_*() functions from reticulate directly, which leads to the deficiencies I described above.

In other words, as things stand now tensorflow implements its own version of py_install() and so diverges in behavior in a bit from reticulate.

[jjallaire]

Oh, okay. @dfalbel I think based on the work that Kevin has just done in py_install() that we should definitely do this. Let me know if you want to discuss.....

[dfalbel]

I agree! The main reason for not completely delegating to py_install were:

1. It didn't create the environment if method="conda" and envname still did not exist.
2. There was no option to destroy and recreate the environment.

Since we plan to use r-reticulate instead of r-tensorflow as the default envname we will stop destroying and re-creating the environment, so 2. may not be important anymore. @kevinushey do you think py_install should also create the conda env (if it doesnt exist)? Edit: just saw it does it in the master version. I'll switch to py_install next week 👍

[dfalbel]

Actually there's also this: https://github.com/rstudio/tensorflow/blob/master/R/install.R#L142-L154
We need to be able to config the python version (since frequently there are problems of compatibility between TensorFlow and the most recent version of python) but install the other packages via pip.

[kevinushey]

Since we plan to use r-reticulate instead of r-tensorflow as the default envname we will stop destroying and re-creating the environment

Rather, I think we should use envname = NULL here which will signal reticulate::py_install() to either use the 'active' environment (as through RETICULATE_PYTHON_ENV) or fall back to r-reticulate.

We need to be able to config the python version (since frequently there are problems of compatibility between TensorFlow and the most recent version of python)

This seems outside the contract of py_install() since it generally assumes that if there's a pre-existing environment it should be used as-is. Any thoughts on how we could handle this? Should we extend py_install() in some way, or perhaps just expose the mechanism used by reticulate to find and choose the environment? (so that tensorflow could decide, as it does now, whether the environment should be re-created)

[skeydan]

Referring to #316 which I just closed, do I understand your above comments correctly @kevinushey - that a user should not do something like this:

# do some work with in my tf_a env
# now without restart or whatever, do
install_tensorflow(method = "...", envname = "tf_b", version = "someotherversion")

Thanks!

[kevinushey]

It's more that if someone were to call:

install_tensorflow(envname = "myexistingenv")

and "myexistingenv" was configured to use an incompatible version of Python, then we'd need to blow away and re-create that environment. However, there isn't currently a good way to express that requirement in py_install() -- that is, if you supply py_install() with an existing environment, it assumes that it is safe to use for whatever package you want to install.

I think the solution is to have reticulate expose the machinery it uses for finding an environment, and provide information about that environment (environment type; version of python; version of pip; and so on). tensorflow could then use that information to decide if it can install into that environment directly, or the environment needs to be recreated.

Because tensorflow needs to take control of the version of Python used in the environment perhaps py_install() isn't the right fit after all.

[jjallaire]

But we really would like to get all of the renv integration and not have special rules and provisos when using tensorflow.....

…

On Fri, Jul 12, 2019 at 2:22 PM Kevin Ushey ***@***.***> wrote: It's more that if someone were to call: install_tensorflow(envname = "myexistingenv") and "myexistingenv" was configured to use an incompatible version of Python, then we'd need to blow away and re-create that environment. However, there isn't currently a good way to express that requirement in py_install() -- that is, if you supply py_install() with an existing environment, it assumes that it is safe to use for whatever package you want to install. I think the solution is to have reticulate expose the machinery it uses for finding an environment, and provide information about that environment (environment type; version of python; version of pip; and so on). tensorflow could then use that information to decide if it can install into that environment directly, or the environment needs to be recreated. Because tensorflow needs to take control of the version of Python used in the environment perhaps py_install() isn't the right fit after all. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#340?email_source=notifications&email_token=AAAZPRYPYPEMYSM3FEKNPBDP7DDWVA5CNFSM4IBZ5IU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ2QHYQ#issuecomment-510985186>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAZPR76QJDIPRW3UMCCH7LP7DDWVANCNFSM4IBZ5IUQ> .

[kevinushey]

@jjallaire can we chat realtime today / next week to see if we can figure out a nice solution here?

[jjallaire]

Not today but definitely next week. How about Monday at 2pm EST?

…

On Fri, Jul 12, 2019 at 2:34 PM Kevin Ushey ***@***.***> wrote: @jjallaire <https://github.com/jjallaire> can we chat realtime today / next week to see if we can figure out a nice solution here? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#340?email_source=notifications&email_token=AAAZPRYDOIBWTR5ODWDBGGDP7DFEXA5CNFSM4IBZ5IU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ2RHIY#issuecomment-510989219>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAZPR2FXOSHMW4ORIZXG33P7DFEXANCNFSM4IBZ5IUQ> .

[kevinushey]

Sounds good!

[dfalbel]

Sorry, I'll be on the plane this Monday.