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

tensorflow should not be listed as dependency in setup.py #546

Closed
amaiya opened this issue Dec 6, 2019 · 8 comments
Closed

tensorflow should not be listed as dependency in setup.py #546

amaiya opened this issue Dec 6, 2019 · 8 comments
Assignees
Labels
external An issue from someone outside the StellarGraph team sg-library

Comments

@amaiya
Copy link

amaiya commented Dec 6, 2019

Describe the bug
The stellargraph library includes tensorflow (i.e., "tensorflow>=1.14,<1.15") as a dependency in setup.py. If a user has tensorflow-gpu installed, then pip3 install stellargraph installs tensorflow, which overwrites things and causes all models to be trained on CPU instead of GPU. To fix, users must remove the tensorflow* installations and reinstall tensorflow-gpu. This was true for TF 1.14 at least, and I think may hold true with TF 2.0.

To Reproduce
Steps to reproduce the behavior:
On a system where tensorflow-gpu is installed, install stellargraph with: pip3 install stellargraph

Expected behavior
Installing stellargraph shouldn't corrupt existing tensorflow-gpu installations.

Solution
The tensorflow entry should be removed as a dependency in setup.py and requirements. Instead, the Installation section in the stellargraph README should include a note about installing tensorflow (or tensorflow-gpu) separately when installing stellargraph. This is how other libraries typically handle it. For instance, see the installation instructions for graph_nets:

To install the Graph Nets library for CPU, run:

$ pip install graph_nets "tensorflow>=1.15,<2" tensorflow_probability

To install the Graph Nets library for GPU, run:

$ pip install graph_nets "tensorflow_gpu>=1.15,<2" tensorflow_probability

In setup.py of graph_nets:

    # Additional "tensorflow" and "tensorflow_probability" requirements should
    # be installed separately (See README).
    install_requires=[
        "absl-py",
        "dm-sonnet<2",
        "future",
        "networkx",
        "numpy",
        "setuptools",
        "six",
    ],
@adocherty
Copy link
Contributor

Hi @amaiya,

Thanks for trying the library! We understand the issue and will look at updating the setup.py file to remove a hard dependency on tensorflow. I am thinking instead of removing tensorflow as a requirement we can add it as a 'extra' requirement, so that you can specify stellargraph either with gpu or cpu support, for example:

To install stellargraph + tensorflow-gpu you can use the following:

pip install stellargraph[gpu]

To install stellargraph + tensorflow you can use the following:

pip install stellargraph[cpu]

Finally, to install stellargraph without change to your existing tensorflow libraries:

pip install stellargraph

However, there should be a warning in the latter case if tensorflow is not installed, or an incompatible version of tensorflow is installed. This will help users who install StellarGraph without reading the fine print so they do not face unexplained errors.

@amaiya
Copy link
Author

amaiya commented Dec 13, 2019

@adocherty Your suggested solution would be great. Thanks to you and your team for a great library - it fills an important gap.

@raulqf
Copy link

raulqf commented Jan 9, 2020

Hi @amaiya,

Thanks for trying the library! We understand the issue and will look at updating the setup.py file to remove a hard dependency on tensorflow. I am thinking instead of removing tensorflow as a requirement we can add it as a 'extra' requirement, so that you can specify stellargraph either with gpu or cpu support, for example:

To install stellargraph + tensorflow-gpu you can use the following:

pip install stellargraph[gpu]

To install stellargraph + tensorflow you can use the following:

pip install stellargraph[cpu]

Finally, to install stellargraph without change to your existing tensorflow libraries:

pip install stellargraph

However, there should be a warning in the latter case if tensorflow is not installed, or an incompatible version of tensorflow is installed. This will help users who install StellarGraph without reading the fine print so they do not face unexplained errors.

Hi all,

I've tried the installation with pip install tensorflow[gpu] but it installed the same version of tensorflow... I finally modified the setup.pu as @amaiya recommended.

huonw added a commit that referenced this issue Jan 10, 2020
)

With tensorflow 2.1, the `tensorflow` package includes GPU support by
default. This seems to be enough to exceed readthedocs's memory limits and cause
the build to fail. `tensorflow-cpu` 2.1 does not hit this problem, and so we can
fall back to that when building documentation.

We probably want to make this more generally configurable (#546), but that's a
more intricate problem, and this gets us back to having working documentation.

This also switches to only using the dependencies in `setup.py`, as building
documentation doesn't need any of the dev dependencies listed in
`requirements.txt`.

See: #602
@kjun9
Copy link
Contributor

kjun9 commented Jan 29, 2020

@raulqf This was originally a problem due to tensorflow having two different packages with either cpu or gpu support. However, with the latest release of the library v0.9.0, stellargraph now depends on tensorflow 2.0, which has CPU and GPU support in one package, so we've decided it's not worth having the different extra options for gpu and cpu specifically.

@amaiya I considered your original suggestion of the ticket "tensorflow should not be listed as dependency", but eventually decided it had too many downsides (see discussion in #567). I don't think this will be an issue now that tensorflow doesn't require a separate gpu package, but do let us know if you still see it being problematic

@kjun9
Copy link
Contributor

kjun9 commented Jan 29, 2020

Actually, it looks like I got the wrong impression from the tensorflow installation docs - the release notes indicate that the cpu + gpu in-one-package support only came about during TF 2.1.0 release https://github.com/tensorflow/tensorflow/releases/tag/v2.1.0

Since stellargraph still depends on TF 2.0, this issue still exists until we upgrade to TF 2.1

@timpitman
Copy link
Contributor

With the move to tensorflow 2.1 in #875, this has now been addressed and will be included in our next release.

@huonw huonw added the external An issue from someone outside the StellarGraph team label Mar 6, 2020
@kjun9
Copy link
Contributor

kjun9 commented Mar 11, 2020

With #1045 done, this should be resolved too

@shauryauppal
Copy link

I am still facing this issue stellar graph is not using GPU, I am working with Watch Your Step Embedding Method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external An issue from someone outside the StellarGraph team sg-library
Projects
None yet
Development

No branches or pull requests

8 participants