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.

Sign up for GitHub

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

Jump to bottom

Fix develop/editable install mode #3074

Merged
merged 6 commits into from
Aug 6, 2021
Merged

Fix develop/editable install mode #3074

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9cb2d64
Fix develop/editable install mode
nvcastet Aug 3, 2021
42f3e19
Add uninstall steps
nvcastet Aug 3, 2021
f169998
Update README with review comments
nvcastet Aug 4, 2021
4eca66e
Modify changelog
nvcastet Aug 4, 2021
ca3f6c2
Fix develop mode in debug mode
nvcastet Aug 5, 2021
ea61648
Address doc review comments
nvcastet Aug 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 15 additions & 7 deletions docs/contributors.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ The versions with CPU support can be installed through the provided :code:`setup

.. code-block:: bash

pip install -e .[dev,test]
pip install .[dev,test]

You can find all other non-Python packages that need to be installed on your system for Horovod to build
in the `Dockerfile.test.cpu <https://github.com/horovod/horovod/blob/master/Dockerfile.test.cpu>`__ and
Expand All @@ -46,26 +46,34 @@ First, uninstall any existing version of Horovod. Be sure to do this *outside*

.. code-block:: bash

$ cd $HOME
$ pip uninstall -y horovod
$ cd -

From *inside* the Horovod root directory, remove any previous build artifacts and then install Horovod:
From *inside* the Horovod root directory, remove any previous build artifacts and then install Horovod in dev mode:

.. code-block:: bash

$ rm -rf build/ dist/
$ HOROVOD_WITH_PYTORCH=1 HOROVOD_WITH_TENSORFLOW=1 python setup.py install
$ HOROVOD_WITH_PYTORCH=1 HOROVOD_WITH_TENSORFLOW=1 pip install -v -e .

Set ``HOROVOD_WITHOUT_[FRAMEWORK]=1`` to disable building Horovod plugins for that framework.
This is useful when you’re testing a feature of one framework in particular and wish to save time.
Set ``HOROVOD_WITH_[FRAMEWORK]=1`` to generate an error if the Horovod plugin for that framework failed to build.
Set ``HOROVOD_GPU_OPERATIONS=NCCL`` to run on GPUs with NCCL.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that.


For a debug build with checked assertions etc. replace the invocation of setup.py by:
In dev 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.

For a debug build with debug symbols, checked assertions etc., use the extra flags below:
maxhgerlach marked this conversation as resolved.
Show resolved Hide resolved

EnricoMi marked this conversation as resolved.
Show resolved Hide resolved
.. code-block:: bash

$ HOROVOD_WITH_PYTORCH=1 HOROVOD_WITH_TENSORFLOW=1 python setup.py build_ext --debug install
$ HOROVOD_WITH_PYTORCH=1 HOROVOD_WITH_TENSORFLOW=1 pip install --global-option build_ext --global-option --debug -v -e .
maxhgerlach marked this conversation as resolved.
Show resolved Hide resolved

To uninstall Horovod installed in dev mode, run the below command from the Horovod root directory:

.. code-block:: bash

$ python setup.py develop -u
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this relate to pip install .[dev,test] above? Should we mention either or both?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@nvcastet nvcastet Aug 4, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


Testing
-------
Expand Down
11 changes: 11 additions & 0 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# ==============================================================================

import os
import shutil
import subprocess
import sys
import textwrap
Expand All @@ -25,6 +26,7 @@

from horovod import __version__

_FRAMEWORK_METADATA_FILE = 'horovod/metadata.json'

class CMakeExtension(Extension):
def __init__(self, name, cmake_lists_dir='.', sources=[], **kwa):
Expand Down Expand Up @@ -52,6 +54,8 @@ def is_build_action():
if sys.argv[1].startswith('install'):
return True

if sys.argv[1].startswith('develop'):
return True

def get_cmake_bin():
return os.environ.get('HOROVOD_CMAKE', 'cmake')
Expand Down Expand Up @@ -98,6 +102,13 @@ def build_extensions(self):
except OSError as e:
raise RuntimeError('CMake failed: {}'.format(str(e)))

if sys.argv[1].startswith('develop'):
# Copy over metadata.json file from build directory
shutil.copyfile(os.path.join(build_dir, _FRAMEWORK_METADATA_FILE),
os.path.join(self.extensions[0].cmake_lists_dir, _FRAMEWORK_METADATA_FILE))
# Remove unfound frameworks, otherwise develop mode will fail the install
self.extensions = [x for x in self.extensions if os.path.exists(self.get_ext_fullpath(x.name))]


# python packages required to use horovod in general
require_list = ['cloudpickle', 'psutil', 'pyyaml', 'dataclasses;python_version<"3.7"']
Expand Down