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

Should the dependency, test_tube, be explicity stated in the readme at the top? #191

Closed
thomasjpfan opened this issue Sep 4, 2019 · 4 comments · Fixed by #205
Closed
Assignees
Labels
question Further information is requested

Comments

@thomasjpfan
Copy link
Contributor

Right after the first example using CoolSystem, the readme starts using test_tube to introduce tensorboard logging. After looking at the setup.py file, it becomes clear that test_tube is a hard dependency.

To reduce cognitive load on readers, it may be good to link to https://github.com/williamFalcon/test-tube right before the code that imports test_tube.

@thomasjpfan thomasjpfan added the question Further information is requested label Sep 4, 2019
@williamFalcon
Copy link
Contributor

good suggestion. Mind submitting a quick PR for that?

@williamFalcon
Copy link
Contributor

been thinking about bringing some of the test-tube stuff into Lightning, but i kind of like the idea of keeping it separate so other libraries can use it. Any thoughts?

@neggert
Copy link
Contributor

neggert commented Sep 5, 2019

IMO the Slurm stuff from test-tube belongs in lightning (or a third entirely separate library). The other functionality would be good to keep separate. I want to take a stab at #47 sometime soon. From that perspective, we'd rather not have any functionality in Lightning that overlaps with MLFlow and friends.

@thomasjpfan
Copy link
Contributor Author

There looks to be bunch of nice features in test-tube that pytorch-lightning takes advantage of. If some of the Slurm stuff were moved here and you fix a bug in test-tube, it would need to be copied here as well.

Another option is to vendor test-tube into the pytorch-lightning library. In the examples, it can look something like:

from pytorch_lightning.test_tube import Experiment
from pytorch_lightning.test_tube.hpr import SlurmCluster

As for support other logging libraries, this may need to directly integrated into test_tube as it can manage logging in a cluster. As you said in #47:

Each call to log needs to be process-safe. Meaning when using distributed only rank=0 will log.

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

Successfully merging a pull request may close this issue.

3 participants