warning if current device index is lower than current local rank

[HelioStrike]

Fixes #1308

Description:

• Display a warning if current device index is lower than local rank. (Show a warning if current device index is lower than current local rank #1308 )

Check list:

• New tests are added (if a new feature is added)
• New doc strings: description and/or example code are in RST format
• Documentation is updated (if required)

[sdesrozis]

@HelioStrike Here some comments.

Could you update the documentation with a note about this feature please ? Thank you again !!

[vfdev-5]

@HelioStrike thanks for the PR. I'd say we have to add tests also for that...

[HelioStrike]

@sdesrozis @vfdev-5 Sorry for asking such a silly question but, what should the tests be checking for, and what should it do? I had a look at test_horovod.py and test_native.py, but I'm still a little confused. :|

[vfdev-5]

@HelioStrike it is good question (not that silly) !

I'd say we have to reproduce initial issue description and test that. Unfortunately, it is not explicitly said in the linked issues.
The problem is here :

import torch
import torch.distributed as dist

def main():

    # !!! We do not call torch.cuda.set_device("cuda:lrank")

    dist.init_process_group(backend="nccl", init_method="env://")
    import os
    local_rank = int(os.environ["LOCAL_RANK"])

    # if we call idist.get_world_size() the first time
    # it will try to setup Native dist comp model
    # and will call all reduce to get some parameter like number of nodes
    # effectively, it does below code:
    # tensor = torch.tensor([local_rank + 1]).to("cuda")
    # dist.all_reduce(tensor, op=dist.ReduceOp.MAX)
    # and this is incorrect as reductions is done on the only one device instead of all participating devices
      
    idist.get_world_size()

    dist.destroy_process_group()

if __name__ == "__main__":
    main()

So, the idea is to check the warning with pytest.warns that idist.get_world_size() raises a warning.
Let me know if you need more details.

[HelioStrike]

@vfdev-5 Thanks, got most of the info I needed!

I added the tests, but the code hangs, so I commented it out for now. I think the ideal behavior for now is for pytest to return true as soon as it sees the warning, instead of executing further. Would you happen to know how this issue can be fixed?

[vfdev-5]

@HelioStrike thanks for the update!

I added the tests, but the code hangs, so I commented it out for now.

Let me give you some explanations of what happens in your test :

    os.environ["RANK"] = "1"
    os.environ["WORLD_SIZE"] = "1"
    os.environ["MASTER_ADDR"] = "0.0.0.0"
    os.environ["MASTER_PORT"] = "2222"
    dist.init_process_group(backend="nccl", init_method="env://")

In this case it is indicated that total number of participating devices/processes (=world size) is 1. It means that distributed communication group will have processes indexed from 0 up to 1 non-included to communicate with.
And current process (=rank) is 1 instead of 0, so dist group waits for rank 0 to appear and thus hangs.

What we would like to check is the following:

• backend: nccl => GPUs
• world size > 1
• user manually creates dist group as above: warning if current device index is lower than current local rank #1335 (comment)

The test setup should be something like here :

ignite/tests/ignite/distributed/comp_models/test_native.py

Lines 252 to 254 in d384dc6

	@pytest.mark.distributed
	@pytest.mark.skipif(torch.cuda.device_count() < 1, reason="Skip if no GPU")
	def test__native_dist_model_create_dist_nccl(local_rank, world_size):

and @pytest.mark.skipif(torch.cuda.device_count() < 2, reason="Skip if less than 1 GPU")

Please, let me know if you have no access to more than 1 GPU. Anyway, here in PRs our CI does not run tests on GPUs but will run once merged to the repository master or any other branch.

[HelioStrike]

@vfdev-5 Thank you for the neat explanation! I'm new to the repo but I think I have a better understanding of things after reading it. I made the suggested changes but I don't have access to 2 GPUs to test it. :(

[vfdev-5]

Thanks for the update

[HelioStrike]

@vfdev-5 Updated! Only issue is that the local_rank fixture only sets up the LOCAL_RANK environment variable, and I still get errors saying that RANK is missing. Made all the other changes.

[vfdev-5]

Only issue is that the local_rank fixture only sets up the LOCAL_RANK environment variable, and I still get errors saying that RANK is missing

@HelioStrike yes, that's true. Actually I want to make the test like here

ignite/tests/ignite/distributed/comp_models/test_native.py

Lines 252 to 254 in d384dc6

	@pytest.mark.distributed
	@pytest.mark.skipif(torch.cuda.device_count() < 1, reason="Skip if no GPU")
	def test__native_dist_model_create_dist_nccl(local_rank, world_size):

which calls

ignite/tests/ignite/distributed/comp_models/test_native.py

Line 208 in d384dc6

def _test__native_dist_model_create_from_context_dist(local_rank, rank, world_size, true_backend, true_device):

The problem with the way you configure ddp group is that the rank is always 1 but it has to be 0 and 1 for different processes.

[HelioStrike]

@vfdev-5 I see now. I updated the code, but is the true_device argument correct (to trigger this specific condition)? Also, like _test__native_dist_model_create_from_context_dist, are there any other cases we'd want to test?

[vfdev-5]

@HelioStrike thanks for working on that. Sorry, I think I badly explained how we would like to test the warning.
Previous links show the example of how to setup distributed configuration and test other things:

• test__native_dist_model_create_dist_nccl : executes tests
• _test__native_dist_model_create_from_context_dist : a subtest inside test__native_dist_model_create_dist_nccl

We would like to write another executable test where we setup distributed conf like above, but we test our particular thing : if idist.get_world_size() raises a warning.

If you'd like to understand all that in detail, you can try to run the test in Colab on a single GPU. It may be a bit tricky to run the test with 2 processes, but you can get a better understanding of how ddp works from user's perspective.
If it is too complicated for you, I can update the test myself later...

[HelioStrike]

@vfdev-5 I see.. I think I got it. I noticed that eventually _test__native_dist_model_create_from_context_dist calls .get_world_size(), so I put that in directly, but yes, I should probably create a new subtest.

[vfdev-5]

@HelioStrike that's true, _test__native_dist_model_create_from_context_dist also calls get world size and probably raises warning too. So, we have to fix that test by adding torch.cuda.set_device("cuda:lrank"). Anyway, I have to check that from my side on 2 GPUs.

[HelioStrike]

@vfdev-5 Sorry about this, I think I lack the knowledge to be able to work with this issue properly...

[vfdev-5]

@HelioStrike no problems, it is a bit a tough subject and requires specific infrastructure to check. I'll finalize this PR later.
Feel free to pick another hactoberfest issue which normally require less particular knowledge etc...

[sdesrozis]

debug print to remove ?

[vfdev-5]

Thanks !

[sdesrozis]

Added a comment. Sorry for the delay...