Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

run par as an entrypoint if there is no patch or jetter patch. #994

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

yikaiMeta
Copy link
Contributor

Summary:

Context:

Currently, when running torchx local job, we are using penv_python as entrypoint. That means we pass the actual .par or .xar file as argument to penv_python. within penv_python, the par/xar is executed as a new process.

Old way to run torchx local job.

For example, if the local job is running "jetter --help", torchx runs it like:
PENV_PAR='/data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/jetter-bin/jetter-bin-inplace.par' penv_python -m jetter.main --help
It passes the par file as an environment variable called "PENV_PAR"(There is another way to pass this to penv_python, which is passing 'PENV_PARNAME' as env variable then get the par file's path using it. But it is very very rare, only 0.1% of total usage.)

New way to run torchx local job

After migration, We will run it like:
PAR_MAIN_OVERRIDE=jetter.main /data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/jetter-bin/jetter-bin-inplace.par --help

NOTE: This diff only migrates one of the most common use cases, which: 1. There are no patch or jetter patch. 2. it's a par not xar. 3. the par file is passed via "PENV_PAR" env variable. For other use cases, we still run penv_python as entrypoint.

Reviewed By: Sanjay-Ganeshan

Differential Revision: D66621649

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 17, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66621649

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66621649

yikaiMeta added a commit to yikaiMeta/torchx that referenced this pull request Dec 17, 2024
…ch#994)

Summary:
Pull Request resolved: pytorch#994

# Context:
Currently, when running torchx local job, we are using penv_python as entrypoint. That means we pass the actual .par or .xar file as argument to penv_python.  within penv_python, the par/xar is executed as a new process.

# Old way to run torchx local job.

For example, if the local job is running "jetter --help", torchx runs it like:
  PENV_PAR='/data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/__jetter-bin__/jetter-bin-inplace.par' penv_python -m jetter.main --help
It passes the par file as an environment variable called "PENV_PAR"(There is another way to pass this to penv_python, which is passing 'PENV_PARNAME' as env variable then get the par file's path using it. But it is very very rare, only 0.1% of total usage.)

# New way to run torchx local job
After migration, We will run it like:
  PAR_MAIN_OVERRIDE=jetter.main /data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/__jetter-bin__/jetter-bin-inplace.par --help

NOTE: This diff only migrates one of the most common use cases, which: 1. There are no patch or jetter patch. 2. it's a par not xar. 3. the par file is passed via "PENV_PAR" env variable.  For other use cases, we still run penv_python as entrypoint.

Reviewed By: Sanjay-Ganeshan

Differential Revision: D66621649
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66621649

yikaiMeta added a commit to yikaiMeta/torchx that referenced this pull request Dec 17, 2024
…ch#994)

Summary:
Pull Request resolved: pytorch#994

# Context:
Currently, when running torchx local job, we are using penv_python as entrypoint. That means we pass the actual .par or .xar file as argument to penv_python.  within penv_python, the par/xar is executed as a new process.

# Old way to run torchx local job.

For example, if the local job is running "jetter --help", torchx runs it like:
  PENV_PAR='/data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/__jetter-bin__/jetter-bin-inplace.par' penv_python -m jetter.main --help
It passes the par file as an environment variable called "PENV_PAR"(There is another way to pass this to penv_python, which is passing 'PENV_PARNAME' as env variable then get the par file's path using it. But it is very very rare, only 0.1% of total usage.)

# New way to run torchx local job
After migration, We will run it like:
  PAR_MAIN_OVERRIDE=jetter.main /data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/__jetter-bin__/jetter-bin-inplace.par --help

NOTE: This diff only migrates one of the most common use cases, which: 1. There are no patch or jetter patch. 2. it's a par not xar. 3. the par file is passed via "PENV_PAR" env variable.  For other use cases, we still run penv_python as entrypoint.

Reviewed By: Sanjay-Ganeshan

Differential Revision: D66621649
yikaiMeta added a commit to yikaiMeta/torchx that referenced this pull request Dec 18, 2024
…ch#994)

Summary:

# Context:
Currently, when running torchx local job, we are using penv_python as entrypoint. That means we pass the actual .par or .xar file as argument to penv_python.  within penv_python, the par/xar is executed as a new process.

# Old way to run torchx local job.

For example, if the local job is running "jetter --help", torchx runs it like:
  PENV_PAR='/data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/__jetter-bin__/jetter-bin-inplace.par' penv_python -m jetter.main --help
It passes the par file as an environment variable called "PENV_PAR"(There is another way to pass this to penv_python, which is passing 'PENV_PARNAME' as env variable then get the par file's path using it. But it is very very rare, only 0.1% of total usage.)

# New way to run torchx local job
After migration, We will run it like:
  PAR_MAIN_OVERRIDE=jetter.main /data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/__jetter-bin__/jetter-bin-inplace.par --help


NOTE: This diff only migrates one of the most common use cases, which: 1. There are no patch or jetter patch. 2. it's a par not xar. 3. the par file is passed via "PENV_PAR" env variable.  For other use cases, we still run penv_python as entrypoint.

Reviewed By: Sanjay-Ganeshan

Differential Revision: D66621649
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66621649

…ch#994)

Summary:

# Context:
Currently, when running torchx local job, we are using penv_python as entrypoint. That means we pass the actual .par or .xar file as argument to penv_python.  within penv_python, the par/xar is executed as a new process.

# Old way to run torchx local job.

For example, if the local job is running "jetter --help", torchx runs it like:
  PENV_PAR='/data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/__jetter-bin__/jetter-bin-inplace.par' penv_python -m jetter.main --help
It passes the par file as an environment variable called "PENV_PAR"(There is another way to pass this to penv_python, which is passing 'PENV_PARNAME' as env variable then get the par file's path using it. But it is very very rare, only 0.1% of total usage.)

# New way to run torchx local job
After migration, We will run it like:
  PAR_MAIN_OVERRIDE=jetter.main /data/users/yikai/fbsource/buck-out/v2/gen/fbcode/a6cb9616985b22b0/jetter/__jetter-bin__/jetter-bin-inplace.par --help


NOTE: This diff only migrates one of the most common use cases, which: 1. There are no patch or jetter patch. 2. it's a par not xar. 3. the par file is passed via "PENV_PAR" env variable.  For other use cases, we still run penv_python as entrypoint.

Reviewed By: Sanjay-Ganeshan

Differential Revision: D66621649
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66621649

Copy link
Contributor

@tonykao8080 tonykao8080 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. there is no behavior change to local scheduler. Just refactoring to the base local_scheduler.

@facebook-github-bot facebook-github-bot merged commit 5e44f20 into pytorch:main Dec 18, 2024
21 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants