Fix develop/editable install mode #3074
Conversation
Signed-off-by: Nicolas Castet <26874160+nvcastet@users.noreply.github.com>
Signed-off-by: Nicolas Castet <26874160+nvcastet@users.noreply.github.com>
docs/contributors.rst
Outdated
|
|
||
| .. code-block:: bash | ||
|
|
||
| $ python setup.py develop -u |
There was a problem hiding this comment.
How does this relate to pip install .[dev,test] above? Should we mention either or both?
There was a problem hiding this comment.
@EnricoMi this uninstall method is when you install the package in dev mode -e since the standard pip uninstall does not work in that case.
The part about running pip install .[dev,test] is a bit redundant. Should we just remove it and modify line 56 with:
HOROVOD_WITH_PYTORCH=1 HOROVOD_WITH_TENSORFLOW=1 pip install -v -e .[dev,test]
So when installing in dev mode, it checks for dev and test dependencies?
There was a problem hiding this comment.
I misread your comment in the changes, python setup.py develop -u is to uninstall an earlier dev installation. What is wrong about pip uninstall horovod?
There was a problem hiding this comment.
The [dev] adds a specific version for all ML frameworks to the dependencies and [test] adds test-specific dependencies as well. So either users install the frameworks in the desired version or simply use [dev] to get recommended versions.
There was a problem hiding this comment.
pip uninstall horovod did not work with the editable mode. But i just tried it using a virtual environment (instead of just the system environment), and it works. I will update the doc since we recommend a venv.
I can mention the dependencies and remove the first call to pip install.
Signed-off-by: Nicolas Castet <26874160+nvcastet@users.noreply.github.com>
Signed-off-by: Nicolas Castet <26874160+nvcastet@users.noreply.github.com>
Signed-off-by: Nicolas Castet <26874160+nvcastet@users.noreply.github.com>
docs/contributors.rst
Outdated
|
|
||
| .. code-block:: bash | ||
| Set ``HOROVOD_GPU_OPERATIONS=NCCL`` to run on GPUs with NCCL. |
There was a problem hiding this comment.
I wouldn't reference any env var other than HOROVOD_WITH[OUT]_[FRAMEWORK] and HOROVOD_DEBUG here, because there are so many. Rather than mentioning the HOROVOD_GPU_OPERATIONS env var here, what about adding a link to https://github.com/horovod/horovod/blob/master/docs/install.rst#environment-variables?
Signed-off-by: Nicolas Castet <26874160+nvcastet@users.noreply.github.com>
| @@ -226,6 +226,7 @@ Optional environment variables that can be set to configure the installation pro | |||
|
|
|||
| Possible values are given in curly brackets: {}. | |||
|
|
|||
| * ``HOROVOD_DEBUG`` - {1}. Install a debug build of Horovod with checked assertions, disabled compiler optimizations etc. | |||
There was a problem hiding this comment.
is the {1}. a copy-paste artefact?
There was a problem hiding this comment.
@EnricoMi No, see the line above and the other environmental variables. This is the possible value for the variable.
|
|
||
| In develop mode, you can edit the Horovod source directly in the repo folder. For Python code, the changes will take effect | ||
| immediately. For **C++/CUDA code**, the ``... pip install -v -e .`` command needs to be invoked again to perform an incremental build. |
There was a problem hiding this comment.
Won't pip skip the installation saying it is already installed?
Checklist before submitting
Description
Fixes develop/editable mode to support direct changes in the source code and incremental C++/CUDA builds.
Note: On the first rebuild, I have noticed the CUDA code being rebuilt when not modified. It is potentially an issue with the add_cuda_library cmake code that generates the dependency file. On the second rebuild, the issue disappears.
This issue may disappear with using cudatoolkit (newer version of cmake) instead of cuda cmake module. Something we could try.
@romerojosh FYI ^^
Fixes #1520.
Review process to land