Fix torchelastic import error due to unsupported signal SIGKILL on Windows

[awaelchli]

Fixes #85427

The signal signal.SIGKILL is not supported on Windows. Since this was included in a type hint, an error is raised directly at import time of distributed/elastic. The fix in this PR proposes the default None and choosing the SIGKILL default only at runtime.

cc @mrshenli @pritamdamania87 @zhaojuanmao @satgera @rohan-varma @gqchen @aazzolini @osalpekar @jiayisuse @H-Huang @kwen2501 @awgu @peterjc123 @mszhanyi @skyline75489 @nbcsm

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/88250

• 📄 Preview Python docs built from this PR
• 📄 Preview C++ docs built from this PR
• ❓ Need help or want to give feedback on the CI? Visit our office hours

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• NVIDIA driver issue

✅ No Failures

As of commit b47d28a:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: awaelchli / name: Adrian Wälchli (f484487)

[H-Huang]

Should this be used instead for windows support?

pytorch/torch/distributed/elastic/multiprocessing/api.py

Line 65 in 8e1ae1c

def _get_kill_signal() -> signal.Signals:

[awaelchli]

Oh awesome! I didn't know this exists. I think we should definitely use this :) Updated!

[H-Huang]

LGTM, thanks for the changes @awaelchli ! Just to confirm, have you validated that this works on Windows? It seems that there is no CI support for distributed/ on Windows so this needs to be manually validated

[awaelchli]

@H-Huang Yes. This is how I have verified it:

Windows 11. Compiled pytorch from source (made sure USE_DISTRIBUTED=1).

(pytorch) C:\Users\Adrian\Documents\pytorch>pip freeze

astunparse==1.6.3
brotlipy==0.7.0
certifi==2022.9.24
cffi @ file:///C:/Windows/Temp/abs_6808y9x40v/croots/recipe/cffi_1659598653989/work
charset-normalizer @ file:///tmp/build/80754af9/charset-normalizer_1630003229654/work
cryptography @ file:///C:/b/abs_36x9ifdcl4/croot/cryptography_1665612655344/work
future @ file:///C:/ci_310/future_1642081234309/work
idna @ file:///C:/b/abs_bdhbebrioa/croot/idna_1666125572046/work
mkl-fft==1.3.1
mkl-random @ file:///C:/ci_310/mkl_random_1643050563308/work
mkl-service==2.4.0
mpmath==1.2.1
networkx==3.0b1
numpy @ file:///C:/b/abs_53f_dbvhzc/croot/numpy_and_numpy_base_1665773185489/work
pycparser @ file:///tmp/build/80754af9/pycparser_1636541352034/work
pyOpenSSL @ file:///opt/conda/conda-bld/pyopenssl_1643788558760/work
PySocks @ file:///C:/ci_310/pysocks_1642089375450/work
PyYAML==6.0
requests @ file:///C:/ci/requests_1657735340829/work
six @ file:///tmp/build/80754af9/six_1644875935023/work
sympy==1.11.1
-e git+https://github.com/awaelchli/pytorch.git@c40033be162db0f94d37e7ccbd2a89d67f8b8e47#egg=torch
typing_extensions @ file:///C:/Windows/TEMP/abs_dd2d0moa85/croots/recipe/typing_extensions_1659638831135/work
urllib3 @ file:///C:/b/abs_a8_3vfznn_/croot/urllib3_1666298943664/work
win-inet-pton @ file:///C:/ci_310/win_inet_pton_1642658466512/work
wincertstore==0.2

(pytorch) C:\Users\Adrian\Documents\pytorch>python -c "import torch; print(torch.__version__)"

1.14.0a0+gitb47d28a

Making sure it reproduces:

(pytorch) C:\Users\Adrian\Documents\pytorch>git checkout master
(pytorch) C:\Users\Adrian\Documents\pytorch>python -c "import torch.distributed.run"

Raises:
AttributeError: module 'signal' has no attribute 'SIGKILL'. Did you mean: 'SIGILL'?
as reported in the linked issue.

Verify fix:

(pytorch) C:\Users\Adrian\Documents\pytorch>git checkout bugfix/signal-sigkill-windows
(pytorch) C:\Users\Adrian\Documents\pytorch>python -c "import torch.distributed.run"

Import succeeds.

[awaelchli]

@H-Huang It looks like there is another PR by @malfet #88522

[awaelchli]

@H-Huang #88522 landed and so there is no longer a need for this PR. I wish there was more coordination here to avoid duplicated work.