fix nproc env in elastic mode for pytorchjob #1948
Conversation
Pull Request Test Coverage Report for Build 6921477150
💛 - Coveralls |
| Value: *pytorchjob.Spec.NprocPerNode, | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
this change removes the necessity to add master spec to run multiple processes per pod using python/torchrun launch method without the elastic spec in the yaml, but doesn't solve #1947
There was a problem hiding this comment.
to solve #1947 I think we need to make changes here
There was a problem hiding this comment.
@deepanker13 elastic.go has env vars when elastic policy is set. As per the original issue, it works correctly when elasticPolicy.NProcPerNode is set.
There was a problem hiding this comment.
@johnugeorge following this pr #1840 , I raised issue #1947 which when implemented should complete the point striked out in the below screenshot
There was a problem hiding this comment.
to solve #1947 I think we need to make changes here
No. this part will be deprecated. We should touch the global one.
|
@kuizhiqing I am bit confused. How does this resolve the case where |
@johnugeorge It is correct if After this PR, |
|
Thanks @kuizhiqing for the explanation. /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johnugeorge, kuizhiqing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
In PyTorchJob elastic mode, nproc env may not be set.
Which issue(s) this PR fixes (optional, in
Fixes #<issue number>, #<issue number>, ...format, will close the issue(s) when PR gets merged):Fixes # #1947
Checklist: