TensorFlow.jl icon indicating copy to clipboard operation
TensorFlow.jl copied to clipboard

using pyimport_conda

Open stevengj opened this issue 5 years ago • 3 comments

I was looking in your build.jl file, and it seems that these lines could be considerably simplified to:

pyimport_conda("tensorflow", "tensorflow=" * cur_py_version, "conda-forge")

This also has the advantage that it will work with non-Conda.jl anaconda installations.

I would often do this lazily in __init__() rather than in the build.jl script, but that is up to you. By the way, you don't need to use Ref for the global PyObjects; instead, do:

const py_tf = PyNULL()


function __init__()
    try
        copy!(py_tf, pyimport("tensorflow"))
        ...

as recommended in the PyCall README.

stevengj avatar Jul 26 '18 11:07 stevengj

Also, libtensorflow_framework.so is installed by Conda… not sure why you need to download it separately? For the GPU?

stevengj avatar Jul 26 '18 14:07 stevengj

Thanks. Unexpected code review gift. Yay :-)

the answer to "Why" in general is that everything used to be different, we used to not use conda, and we used to ship a custom libtensorflow binary, and at one point our python version and our libtensorflow doesn't even agree.

In particular re: seperate libtensorflow. Even now it is the part one often wants to replace; being performance critical As you said, you need to do so for GPU. (there is no GPU compatible thing on conda-forge). You also want to do so for different architectures. Ideally we'ld be downloading prebuilt ones from somewhere, but only the generic (slower) ones are available now AFAICT.

In particular re: using pyimport_conda. This largely comes down to me not fully understanding about the 2nd and 3rd arguments. I assume the 3rd will automatically add the conda-forge channel? That change can be made. Though I think It would be best to do at build time, just because our first run time is already huge because julia parsing everything, and I'ld rather not make it longer.

Re: using PyNull, we should look into that.

Thanks a lot

oxinabox avatar Jul 27 '18 02:07 oxinabox

Yes, 3rd arg adds the channel.

stevengj avatar Jul 27 '18 02:07 stevengj