Skip reconciliate_processes if used within a cluster environment that creates processes externally

[ananthsub]

What does this PR do?

Fixes #9388

Does your PR introduce any breaking changes? If yes, please list them.

Before submitting

• Was this discussed/approved via a GitHub issue? (not for typos and docs)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure your PR does only one thing, instead of bundling different changes together?
• Did you make sure to update the documentation with your changes? (if necessary)
• Did you write any new necessary tests? (not for typos and docs)
• Did you verify new and existing tests pass locally with your changes?
• Did you list all the breaking changes introduced by this pull request?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

• Is this pull request ready for review? (if not, please submit in draft mode)
• Check that all items from Before submitting are resolved
• 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 🙃

[awaelchli]

req. review from @tchaton

[codecov]

Codecov Report

Merging #9389 (d1804bc) into master (81687aa) will decrease coverage by 4%.
The diff coverage is 0%.

❗ Current head d1804bc differs from pull request most recent head 93b389a. Consider uploading reports for the commit 93b389a to get more accurate results

@@           Coverage Diff           @@
##           master   #9389    +/-   ##
=======================================
- Coverage      93%     89%    -4%     
=======================================
  Files         180     180            
  Lines       14965   15065   +100     
=======================================
- Hits        13904   13370   -534     
- Misses       1061    1695   +634     

[tchaton]

LGTM !

[ananthsub]

@tchaton @awaelchli I'm not sure why this test is failing:

• https://github.com/PyTorchLightning/pytorch-lightning/blob/15434a9c357745c8f6df5739c6a423bbb03787ad/tests/trainer/test_trainer.py#L1810-L1834
• https://dev.azure.com/PytorchLightning/pytorch-lightning/_build/results?buildId=32253&view=logs&j=3afc50db-e620-5b81-6016-870a6976ad29&t=d9f671c5-a304-5675-5394-961fd7f98b9b

are special tests run with torch.distributed.run or torchelastic somehow? it doesn't seem like this is the case with https://github.com/PyTorchLightning/pytorch-lightning/blob/15434a9c357745c8f6df5739c6a423bbb03787ad/tests/special_tests.sh#L20-L65

[tchaton]

Hey @ananthsub. Interesting. Mind taking CustomException out of the test ? The test should run as special one, so without TorchElastic.

[ananthsub]

ah @tchaton @awaelchli - I believe it's due to the check in the LightningEnvironment here: https://github.com/PyTorchLightning/pytorch-lightning/blob/b294c5760eee30a995fcd400127be209b12c4d7c/pytorch_lightning/plugins/environments/lightning_environment.py#L43-L49

LOCAL_RANK is added to the environment here: https://github.com/PyTorchLightning/pytorch-lightning/blob/b294c5760eee30a995fcd400127be209b12c4d7c/pytorch_lightning/plugins/training_type/ddp.py#L198

So while cluster_environment.creates_children() is False at the beginning of the script, it is True at the point of deadlock detection. This leads to the deadlock detection being disabled, causing the test to fail.

This was added in #7480

[awaelchli]

So while cluster_environment.creates_children() is False at the beginning of the script, it is True at the point of deadlock detection. This leads to the deadlock detection being disabled, causing the test to fail.

This was added in #7480

You are right, I should have captured the value like you do in this PR

[tchaton]

Hey @ananthsub,

Do you see blockers to introduce an env variable to enable Lightning to kill the processes even with a cluster env.

Something like PL_PREVENT_DEADLOCK_MECHANISM='1'

I believe this would be useful for Grid.ai.

Best,
T.C

[ananthsub]

Hey @ananthsub,

Do you see blockers to introduce an env variable to enable Lightning to kill the processes even with a cluster env.

Something like PL_PREVENT_DEADLOCK_MECHANISM='1'

I believe this would be useful for Grid.ai.

Best,
T.C

@tchaton @awaelchli could you take another look to confirm the opt-in deadlock detection looks reasonable?

[tchaton]

LGTM !