Repository: Lightning-AI/pytorch-lightning
Pull request: Fix _module_available to detect horovod.torch properly
## What does this PR do?

<!--
Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.

If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

The following links the related issue to the PR (https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)
-->

Fixes #12370, such that horovod.torch is again properly detected.

This PR simplifies the logic of the _module_available function. I think in general it's not a good idea to try to deviate too much from importlib when testing for module availability, if the specific logic of `hasattr` is necessary for any check it would be better to handle that case separately. I tried reproducing the error that first triggered this change in the torchmetrics repo https://github.com/PyTorchLightning/metrics/issues/770 by testing `_module_available("transformers.models.auto")` and this didn't give me any problems, but a maintainer might be a better judge of this. I can replicate this code to that repo later.

Note I run horovod in develop mode not sure if that's related, but latest pypi version of horovod has a very restrictive pin on the pytorch lightning version.

### Does your PR introduce any breaking changes? If yes, please list them.

<!-- FILL IN or None -->

## Before submitting

- [ ] Was this **discussed/approved** via a GitHub issue? (not for typos and docs)
- [x] Did you read the [contributor guideline](https://github.com/PyTorchLightning/pytorch-lightning/blob/master/.github/CONTRIBUTING.md), **Pull Request** section?
- [x] Did you make sure your **PR does only one thing**, instead of bundling different changes together?
- [x] Did you make sure to **update the documentation** with your changes? (if necessary)
- [x] Did you write any **new necessary tests**? (not for typos and docs)
- [x] Did you verify new and **existing tests pass** locally with your changes?
- [x] Did you list all the **breaking changes** introduced by this pull request?
- [x] Did you **update the [CHANGELOG](https://github.com/PyTorchLightning/pytorch-lightning/blob/master/CHANGELOG.md)**? (not for typos, docs, test updates, or internal minor changes/refactorings)

<!-- In the CHANGELOG, separate each item in the unreleased section by a blank line to reduce collisions -->

## PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read [Review guidelines](https://github.com/PyTorchLightning/pytorch-lightning/wiki/Review-guidelines). In short, see the following bullet-list:

- [x] Is this pull request ready for review? (if not, please submit in draft mode)
- [x] Check that all items from **Before submitting** are resolved
- [x] Make sure the title is self-explanatory and the description concisely explains the PR
- [ ] Add labels and milestones (and optionally projects) to the PR so it can be classified

## Did you have fun?

Make sure you had fun coding 🙃


Problem statement:
_module_available does not correctly check child modules
## 🐛 Bug

I tried to use horovod accelerator which is correctly installed in my environment, as I can `import horovod.torch` without any problem but I get an error stating that horovod is not installed. I checked the source code to see how module availability was checked and was surprised to see that lightning uses `hasattr` to check child modules. I guess it comes handy when you are not trying to check for a module but for a function or a class, but I don't understand why the function does not simply try to call `import horovod.torch` (which works) instead of `hasattr(horovod, "torch")` (which returns `False`). Indeed, a submodule is not necessarily an attribute of its parent module unless stated explicitly, which is apparently not the case for horovod. If you still want to handle the case where you are trying to import an object and not a module, you can check for a `ModuleNotFoundError` and then use `hasattr` instead.

### To Reproduce

With horovod=0.24.1 installed with PyTorch compatibility, use `_module_available("horovod.torch")` (or simply `import horovod; hasattr(horovod, "torch")`). 

### Expected behavior

The function should return `True`. It has to be noted that if a `import horovod.torch` has been performed prior to the function call, it returns `True`, which can serve as a workaround in the meantime. 

### Environment

PyTorch version: 1.10.1
Is debug build: False
CUDA used to build PyTorch: 11.3
ROCM used to build PyTorch: N/A

OS: CentOS Stream release 8 (x86_64)
GCC version: (GCC) 8.5.0 20210514 (Red Hat 8.5.0-10)
Clang version: Could not collect
CMake version: version 3.20.2
Libc version: glibc-2.28

Python version: 3.9.7 (default, Sep 16 2021, 13:09:58)  [GCC 7.5.0] (64-bit runtime)
Python platform: Linux-4.18.0-365.el8.x86_64-x86_64-with-glibc2.28
Is CUDA available: True
CUDA runtime version: 11.6.55
GPU models and configuration:
GPU 0: Tesla P100-PCIE-16GB
GPU 1: Tesla P100-PCIE-16GB

Nvidia driver version: 510.47.03
cuDNN version: Probably one of the following:
/usr/lib64/libcudnn.so.8.3.2
/usr/lib64/libcudnn_adv_infer.so.8.3.2
/usr/lib64/libcudnn_adv_train.so.8.3.2
/usr/lib64/libcudnn_cnn_infer.so.8.3.2
/usr/lib64/libcudnn_cnn_train.so.8.3.2
/usr/lib64/libcudnn_ops_infer.so.8.3.2
/usr/lib64/libcudnn_ops_train.so.8.3.2
HIP runtime version: N/A
MIOpen runtime version: N/A

Versions of relevant libraries:
[pip3] mypy-extensions==0.4.3
[pip3] numpy==1.22.1
[pip3] pytorch-lightning==1.5.10
[pip3] torch==1.10.1
[pip3] torchaudio==0.10.1
[pip3] torchmetrics==0.7.2
[pip3] torchvision==0.11.2
[pip3] horovod==0.24.1
[conda] blas                      1.0                         mkl
[conda] cudatoolkit               11.3.1               h2bc3f7f_2
[conda] mkl                       2021.4.0           h06a4308_640
[conda] mkl-service               2.4.0            py39h7f8727e_0
[conda] mkl_fft                   1.3.1            py39hd3c417c_0
[conda] mkl_random                1.2.2            py39h51133e4_0
[conda] mypy-extensions           0.4.3                    pypi_0    pypi
[conda] numpy                     1.22.1                   pypi_0    pypi
[conda] numpy-base                1.21.2           py39h79a1101_0
[conda] pytorch                   1.10.1          py3.9_cuda11.3_cudnn8.2.0_0    pytorch
[conda] pytorch-lightning         1.5.10                   pypi_0    pypi
[conda] pytorch-mutex             1.0                        cuda    pytorch
[conda] torchaudio                0.10.1               py39_cu113    pytorch
[conda] torchmetrics              0.7.2                    pypi_0    pypi
[conda] torchvision               0.11.2               py39_cu113    pytorch


### Additional context

If you confirm that this is unexpected behavior, I can write a PR that should fix this by directly trying to import the module.


cc @awaelchli

Hints from issue discussion:

Commit message: Merge branch 'master' into simplify-module-checks

Patch to review:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0126950f2697b..864c8daee7133 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -923,6 +923,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 - Fixed initializing optimizers unnecessarily in `DDPFullyShardedStrategy` ([#12267](https://github.com/PyTorchLightning/pytorch-lightning/pull/12267))
 
+- Fixed check for horovod module ([#12377](https://github.com/PyTorchLightning/pytorch-lightning/pull/12377))
 
 ## [1.5.10] - 2022-02-08
 
diff --git a/pytorch_lightning/trainer/connectors/accelerator_connector.py b/pytorch_lightning/trainer/connectors/accelerator_connector.py
index 673964b13e5ab..90f0d139e7416 100644
--- a/pytorch_lightning/trainer/connectors/accelerator_connector.py
+++ b/pytorch_lightning/trainer/connectors/accelerator_connector.py
@@ -625,7 +625,7 @@ def _handle_horovod(self) -> None:
         hvd.init()
         if isinstance(self.accelerator, GPUAccelerator):
             # Horovod assigns one local GPU per process
-            self._parallel_devices = list(range(hvd.local_size()))
+            self._parallel_devices = [torch.device(f"cuda:{i}") for i in range(hvd.local_size())]
         else:
             self._parallel_devices = [torch.device("cpu")] * hvd.local_size()
 
diff --git a/pytorch_lightning/utilities/imports.py b/pytorch_lightning/utilities/imports.py
index c98874bcbd742..5ef7c086cb714 100644
--- a/pytorch_lightning/utilities/imports.py
+++ b/pytorch_lightning/utilities/imports.py
@@ -53,13 +53,9 @@ def _module_available(module_path: str) -> bool:
     if not _package_available(module_names[0]):
         return False
     try:
-        module = importlib.import_module(module_names[0])
-    except ImportError:
+        importlib.import_module(module_path)
+    except ModuleNotFoundError:
         return False
-    for name in module_names[1:]:
-        if not hasattr(module, name):
-            return False
-        module = getattr(module, name)
     return True
 
 
diff --git a/tests/models/data/horovod/train_default_model.py b/tests/models/data/horovod/train_default_model.py
index 4b8974a5ce5b7..ab692d379a7f2 100644
--- a/tests/models/data/horovod/train_default_model.py
+++ b/tests/models/data/horovod/train_default_model.py
@@ -42,9 +42,10 @@
 parser = argparse.ArgumentParser()
 parser.add_argument("--trainer-options", required=True)
 parser.add_argument("--on-gpu", action="store_true", default=False)
+parser.add_argument("--check-size", action="store_true", default=False)
 
 
-def run_test_from_config(trainer_options, on_gpu, check_size=True):
+def run_test_from_config(trainer_options, on_gpu, check_size):
     """Trains the default model with the given config."""
     set_random_main_port()
     reset_seed()
@@ -107,4 +108,4 @@ def training_epoch_end(self, outputs) -> None:
 
 if __name__ == "__main__":
     args = parser.parse_args()
-    run_test_from_config(json.loads(args.trainer_options), args.on_gpu)
+    run_test_from_config(json.loads(args.trainer_options), args.on_gpu, args.check_size)
diff --git a/tests/models/test_horovod.py b/tests/models/test_horovod.py
index 5d553394fed9c..f0dd5c82ba51f 100644
--- a/tests/models/test_horovod.py
+++ b/tests/models/test_horovod.py
@@ -41,7 +41,6 @@
 
 
 @RunIf(min_gpus=1, horovod=True)
-@pytest.mark.xfail(reason="FIXME(@Borda): nccl is not available in the GPU image")
 def test_nccl_is_available_on_gpu_environment():
     from tests.helpers.runif import _HOROVOD_NCCL_AVAILABLE
 
@@ -71,6 +70,8 @@ def _run_horovod(trainer_options):
     ]
     if trainer_options.get("accelerator", "cpu") == "gpu":
         cmdline += ["--on-gpu"]
+    if devices == 2:
+        cmdline += ["--check-size"]
     exit_code = subprocess.call(" ".join(cmdline), shell=True, env=os.environ.copy())
     assert exit_code == 0
 
@@ -93,7 +94,7 @@ def test_horovod_cpu(tmpdir):
 @RunIf(skip_windows=True, horovod=True, skip_49370=True)
 def test_horovod_cpu_accumulate_grad_batches(tmpdir):
     trainer_options = dict(
-        default_root_dir=tmpdir,
+        default_root_dir=str(tmpdir),
         enable_progress_bar=False,
         max_epochs=1,
         limit_train_batches=4,
@@ -154,7 +155,7 @@ def test_horovod_multi_gpu(tmpdir):
 @RunIf(min_gpus=2, skip_windows=True, horovod_nccl=True)
 def test_horovod_multi_gpu_accumulate_grad_batches(tmpdir):
     trainer_options = dict(
-        default_root_dir=tmpdir,
+        default_root_dir=str(tmpdir),
         enable_progress_bar=False,
         max_epochs=1,
         limit_train_batches=4,
diff --git a/tests/utilities/test_imports.py b/tests/utilities/test_imports.py
index 75bcb51ffb89f..8f1937d0512eb 100644
--- a/tests/utilities/test_imports.py
+++ b/tests/utilities/test_imports.py
@@ -13,7 +13,16 @@
 # limitations under the License.
 import operator
 
-from pytorch_lightning.utilities import _module_available
+from pytorch_lightning.utilities import (
+    _APEX_AVAILABLE,
+    _BAGUA_AVAILABLE,
+    _DEEPSPEED_AVAILABLE,
+    _FAIRSCALE_AVAILABLE,
+    _HOROVOD_AVAILABLE,
+    _module_available,
+    _OMEGACONF_AVAILABLE,
+    _POPTORCH_AVAILABLE,
+)
 from pytorch_lightning.utilities.imports import _compare_version
 
 
@@ -45,3 +54,48 @@ def test_compare_version(monkeypatch):
     assert not _compare_version("torch", operator.ge, "1.10.0.rc0")
     assert _compare_version("torch", operator.ge, "1.10.0", use_base_version=True)
     assert not _compare_version("torch", operator.ge, "1.10.0")
+
+
+def test_imports():
+    try:
+        import apex  # noqa
+
+        assert _APEX_AVAILABLE
+    except ModuleNotFoundError:
+        assert not _APEX_AVAILABLE
+    try:
+        import bagua  # noqa
+
+        assert _BAGUA_AVAILABLE
+    except ModuleNotFoundError:
+        assert not _BAGUA_AVAILABLE
+    try:
+        import deepspeed  # noqa
+
+        assert _DEEPSPEED_AVAILABLE
+    except ModuleNotFoundError:
+        assert not _DEEPSPEED_AVAILABLE
+    try:
+        import fairscale.nn  # noqa
+
+        assert _FAIRSCALE_AVAILABLE
+    except ModuleNotFoundError:
+        assert not _FAIRSCALE_AVAILABLE
+    try:
+        import horovod.torch  # noqa
+
+        assert _HOROVOD_AVAILABLE
+    except ModuleNotFoundError:
+        assert not _HOROVOD_AVAILABLE
+    try:
+        import omegaconf  # noqa
+
+        assert _OMEGACONF_AVAILABLE
+    except ModuleNotFoundError:
+        assert not _OMEGACONF_AVAILABLE
+    try:
+        import poptorch  # noqa
+
+        assert _POPTORCH_AVAILABLE
+    except ModuleNotFoundError:
+        assert not _POPTORCH_AVAILABLE


Code review comments:
diff_hunk: @@ -45,3 +54,48 @@ def test_compare_version(monkeypatch):
     assert not _compare_version("torch", operator.ge, "1.10.0.rc0")
     assert _compare_version("torch", operator.ge, "1.10.0", use_base_version=True)
     assert not _compare_version("torch", operator.ge, "1.10.0")
+
+
+def test_imports():
+    try:
+        import apex  # noqa
+
+        assert _APEX_AVAILABLE
+    except ModuleNotFoundError:
+        assert not _APEX_AVAILABLE
path: tests/utilities/test_imports.py
text: @user1:
```suggestion
    try:
        import apex  # noqa
    except ModuleNotFoundError:
        assert not _APEX_AVAILABLE
    else:
        assert _APEX_AVAILABLE

```
lest move it to `else` https://note.nkmk.me/en/python-try-except-else-finally/