Modify LSFEnvironment to use more reliable environment variable #10825

ajtritt · 2021-11-29T22:42:47Z

The environment variable LSB_HOSTS was being used to determine node rank and master address. This is not always populated. This PR updates LSFEnvironment to use LSB_DJOB_RANKFILE instead.

Fix #10216

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 🙃

- Use LSB_DJOB_RANKFILE to determine node rank and master address - Update tests

for more information, see https://pre-commit.ci

- Update CHANGELOG - Fix PEP8 violations

- Change master_address and master_port to main_address and main_port - Fix errors in mocks and tests

… into bugfix/lsf_env

for more information, see https://pre-commit.ci

… into bugfix/lsf_env

- this method never existed on ClusterEnvironment

pytorch_lightning/plugins/environments/lsf_environment.py

… into bugfix/lsf_env

for more information, see https://pre-commit.ci

pytorch_lightning/plugins/environments/lsf_environment.py

stale · 2022-01-03T20:28:24Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you need further help see our docs: https://pytorch-lightning.readthedocs.io/en/latest/generated/CONTRIBUTING.html#pull-request or ask the assistance of a core contributor here or on Slack. Thank you for your contributions.

amorehead · 2022-01-12T03:49:58Z

Out of curiosity, which release branch for Lightning are these changes expected to be released? 1.5.9?

Co-authored-by: thomas chaton <thomas@grid.ai> Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Andrew Tritt <andrew.j.tritt@gmail.com>

awaelchli · 2022-01-16T02:12:10Z

Hello
Yes, I will try to include the fix in 1.5.9 #11421. You can also install directly from master go get this feature/fix.

Co-authored-by: thomas chaton <thomas@grid.ai> Co-authored-by: Carlos Mocholí <carlossmocholi@gmail.com> Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com> Co-authored-by: Andrew Tritt <andrew.j.tritt@gmail.com>

ajtritt and others added 3 commits November 29, 2021 17:37

Update LSFEnvironment

8e3d633

- Use LSB_DJOB_RANKFILE to determine node rank and master address - Update tests

[pre-commit.ci] auto fixes from pre-commit.com hooks

f970ee4

for more information, see https://pre-commit.ci

Cleanup for review

d0b6796

- Update CHANGELOG - Fix PEP8 violations

ajtritt changed the title ~~Update LSFEnvironment~~ Modify LSFEnvironment to use more reliable environment variable Nov 29, 2021

ajtritt and others added 3 commits November 29, 2021 19:04

Work out kinks

480c23b

- Change master_address and master_port to main_address and main_port - Fix errors in mocks and tests

Merge branch 'bugfix/lsf_env' of github.com:ajtritt/pytorch-lightning…

cd45898

… into bugfix/lsf_env

[pre-commit.ci] auto fixes from pre-commit.com hooks

d9440ce

for more information, see https://pre-commit.ci

carmocca added environment: lsf feature Is an improvement or enhancement labels Nov 30, 2021

ajtritt added 4 commits November 30, 2021 11:31

Fix deprecation test

ba3a4a8

Merge branch 'bugfix/lsf_env' of github.com:ajtritt/pytorch-lightning…

ae740ed

… into bugfix/lsf_env

Remove is_using_lsf from deprecated tests

8180e15

- this method never existed on ClusterEnvironment

fix pep8 errors

87a8234

ajtritt marked this pull request as ready for review November 30, 2021 20:31

ajtritt requested review from awaelchli, Borda, carmocca, justusschock, kaushikb11, rohitgr7, SeanNaren, tchaton and williamFalcon as code owners November 30, 2021 20:31

carmocca added environment: lsf and removed environment: lsf labels Nov 30, 2021

tchaton reviewed Dec 1, 2021

View reviewed changes

pytorch_lightning/plugins/environments/lsf_environment.py Outdated Show resolved Hide resolved

ajtritt and others added 3 commits December 1, 2021 12:26

Add is_using_lsf deprecation warning

4d376c0

Merge branch 'bugfix/lsf_env' of github.com:ajtritt/pytorch-lightning…

2302c5c

… into bugfix/lsf_env

[pre-commit.ci] auto fixes from pre-commit.com hooks

2f90f13

for more information, see https://pre-commit.ci

mergify bot added the has conflicts label Dec 2, 2021

awaelchli and others added 4 commits December 20, 2021 19:19

update docstrings

7e39a34

compress message

943fcaa

merge

71314b9

[pre-commit.ci] auto fixes from pre-commit.com hooks

45fb87c

for more information, see https://pre-commit.ci

awaelchli reviewed Dec 20, 2021

View reviewed changes

pytorch_lightning/plugins/environments/lsf_environment.py Outdated Show resolved Hide resolved

pytorch_lightning/plugins/environments/lsf_environment.py Outdated Show resolved Hide resolved

stale bot added the won't fix This will not be worked on label Jan 3, 2022

awaelchli removed the won't fix This will not be worked on label Jan 4, 2022

awaelchli modified the milestones: 1.6, 1.5.x Jan 4, 2022

tchaton requested review from awaelchli and carmocca January 4, 2022 10:49

awaelchli added 2 commits January 5, 2022 11:23

drop string representation

a1d5f49

move environment variable lookup to property getters

dd88292

awaelchli force-pushed the bugfix/lsf_env branch from 9d0bf5a to dd88292 Compare January 5, 2022 10:31

awaelchli approved these changes Jan 5, 2022

View reviewed changes

mergify bot added the ready PRs ready to be merged label Jan 5, 2022

awaelchli enabled auto-merge (squash) January 5, 2022 10:34

Merge branch 'master' into bugfix/lsf_env

d156ffa

kaushikb11 approved these changes Jan 5, 2022

View reviewed changes

awaelchli added 2 commits January 5, 2022 11:49

use fsspec io to open file

9888945

set read mode for fs.open

630dd6f

awaelchli merged commit dbf1acd into Lightning-AI:master Jan 5, 2022

rohitgr7 mentioned this pull request Feb 7, 2022

Weekly Patch Release v1.5.10 #11789

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify LSFEnvironment to use more reliable environment variable #10825

Modify LSFEnvironment to use more reliable environment variable #10825

ajtritt commented Nov 29, 2021 •

edited

Loading

stale bot commented Jan 3, 2022

amorehead commented Jan 12, 2022

awaelchli commented Jan 16, 2022

Modify LSFEnvironment to use more reliable environment variable #10825

Modify LSFEnvironment to use more reliable environment variable #10825

Conversation

ajtritt commented Nov 29, 2021 • edited Loading

Before submitting

PR review

Did you have fun?

stale bot commented Jan 3, 2022

amorehead commented Jan 12, 2022

awaelchli commented Jan 16, 2022

ajtritt commented Nov 29, 2021 •

edited

Loading