Repository: pytorch/ignite

Issue:
Improve Horovod comp model
## 🚀 Feature

According to https://github.com/pytorch/ignite/pull/1433#discussion_r516992540 there is an incoherent model creation for horovod backend.


Discussion:

Patch:
diff --git a/ignite/distributed/comp_models/horovod.py b/ignite/distributed/comp_models/horovod.py
index 422eb329..d77464d1 100644
--- a/ignite/distributed/comp_models/horovod.py
+++ b/ignite/distributed/comp_models/horovod.py
@@ -44,41 +44,48 @@ if has_hvd_support:
         @staticmethod
         def create_from_context() -> Optional["_HorovodDistModel"]:
             rank = _HorovodDistModel._get_hvd_rank()
-            if not (has_hvd_support and rank > -1):
+            # hvd must be initialized
+            if not rank > -1:
                 return None
             return _HorovodDistModel()
 
         @staticmethod
-        def create_from_backend(backend: str, **kwargs: Any) -> "_HorovodDistModel":
+        def create_from_backend(backend: str = HOROVOD, **kwargs: Any) -> "_HorovodDistModel":
             if backend not in _HorovodDistModel.available_backends:
                 raise ValueError(f"Backend should be one of '{_HorovodDistModel.available_backends}'")
 
             rank = _HorovodDistModel._get_hvd_rank()
-            if has_hvd_support and rank > -1:
+            # hvd must be not initialized
+            if rank > -1:
                 raise RuntimeError("Can not re-initialize Horovod if it is already initialized")
-            return _HorovodDistModel(do_init=True, **kwargs)
+            return _HorovodDistModel(backend, **kwargs)
 
-        def __init__(self, do_init: bool = False, **kwargs: Any) -> None:
+        def __init__(self, backend: Optional[str] = None, **kwargs: Any) -> None:
             """This is a private method. Please, use `create_from_backend` or `create_from_context`
             """
             super(_HorovodDistModel, self).__init__()
-            self._backend = HOROVOD  # type: str
-            if do_init:
-                comm = kwargs.get("comm", None)
-                hvd.init(comm=comm)
-
-            self._local_rank = hvd.local_rank()
-
-            if do_init and torch.cuda.is_available():
-                torch.cuda.set_device(self._local_rank)
+            if backend is not None:
+                self._create_from_backend(backend, **kwargs)
+            else:
+                self._init_from_context()
+
+        def _create_from_backend(self, backend: str, **kwargs: Any) -> None:
+            self._backend = backend  # type: str
+            comm = kwargs.get("comm", None)
+            hvd.init(comm=comm)
+            self._setup_attrs()
+            if torch.cuda.is_available():
+                torch.cuda.set_device(self.get_local_rank())
 
+        def _init_from_context(self) -> None:
+            self._backend = HOROVOD
             self._setup_attrs()
 
         def _compute_nproc_per_node(self) -> int:
             return hvd.local_size()
 
         def get_local_rank(self) -> int:
-            return self._local_rank
+            return hvd.local_rank()
 
         def get_rank(self) -> int:
             return hvd.rank()
diff --git a/ignite/distributed/comp_models/native.py b/ignite/distributed/comp_models/native.py
index 1857185e..d4fb17eb 100644
--- a/ignite/distributed/comp_models/native.py
+++ b/ignite/distributed/comp_models/native.py
@@ -47,6 +47,9 @@ if has_native_dist_support:
 
         @staticmethod
         def create_from_backend(backend: str, **kwargs: Any) -> "_NativeDistModel":
+            if backend not in _NativeDistModel.available_backends:
+                raise ValueError(f"Backend should be one of '{_NativeDistModel.available_backends}'")
+
             if dist.is_available() and dist.is_initialized():
                 raise RuntimeError("Can not create new distributed process group if default one is already initialized")
             return _NativeDistModel(backend=backend, **kwargs)
diff --git a/ignite/distributed/comp_models/xla.py b/ignite/distributed/comp_models/xla.py
index 9e1f64c8..58feca3d 100644
--- a/ignite/distributed/comp_models/xla.py
+++ b/ignite/distributed/comp_models/xla.py
@@ -39,6 +39,9 @@ if has_xla_support:
 
         @staticmethod
         def create_from_backend(backend: str = XLA_TPU, **kwargs: Any) -> "_XlaDistModel":
+            if backend not in _XlaDistModel.available_backends:
+                raise ValueError(f"Backend should be one of '{_XlaDistModel.available_backends}'")
+
             return _XlaDistModel(backend=backend, **kwargs)
 
         def __init__(self, backend: Optional[str] = None, **kwargs: Any):
diff --git a/ignite/distributed/utils.py b/ignite/distributed/utils.py
index 5a144f27..7d7809b9 100644
--- a/ignite/distributed/utils.py
+++ b/ignite/distributed/utils.py
@@ -496,7 +496,7 @@ def initialize(backend: str, **kwargs: Any) -> None:
     for comp_model_cls in registered_computation_models:
         if backend not in comp_model_cls.available_backends:
             continue
-        _set_model(comp_model_cls(backend, **kwargs))  # type: ignore[arg-type]
+        _set_model(comp_model_cls(backend, **kwargs))
 
 
 def finalize() -> None:


Test patch:
diff --git a/tests/ignite/distributed/comp_models/test_native.py b/tests/ignite/distributed/comp_models/test_native.py
index 3a1e5b67..e7121fb1 100644
--- a/tests/ignite/distributed/comp_models/test_native.py
+++ b/tests/ignite/distributed/comp_models/test_native.py
@@ -31,6 +31,9 @@ def test__native_dist_model():
     else:
         assert "mpi" not in available_backends
 
+    with pytest.raises(ValueError, match=r"Backend should be one of"):
+        _NativeDistModel.create_from_backend("abc")
+
 
 @pytest.mark.distributed
 @pytest.mark.skipif(not dist.is_nccl_available(), reason="Skip if nccl not available")
diff --git a/tests/ignite/distributed/comp_models/test_xla.py b/tests/ignite/distributed/comp_models/test_xla.py
index fd166e74..ea6fc8f1 100644
--- a/tests/ignite/distributed/comp_models/test_xla.py
+++ b/tests/ignite/distributed/comp_models/test_xla.py
@@ -17,6 +17,9 @@ def test__xla_model():
     available_backends = _XlaDistModel.available_backends
     assert "xla-tpu" in available_backends
 
+    with pytest.raises(ValueError, match=r"Backend should be one of"):
+        _XlaDistModel.create_from_backend("abc")
+
 
 def _test_xla_spawn_fn(local_rank, world_size, device):
     from ignite.distributed.utils import _model