Skip to content

Conversation

@bobyangyf
Copy link
Contributor

Summary:
Parameterize Scheduler
This not only more precisely expresses it's type (previously was unspecified generic variant of AppDryRunInfo[T] where T was unspecified; not it can be fully specified); This also allows us to inject PipelineDryRunInfo for schedulers that are pipeline schedulers

Differential Revision: D71023681

@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 Mar 21, 2025
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

@kiukchung kiukchung left a comment

Choose a reason for hiding this comment

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

I don't fully understand this change. It seems like you want torchx.schedulers.api.Scheduler interface to be able to deal with both AppDef and PipelineDef in the same API. For instance:

def schedule(desc: Union[AppDef, PipelineDef], ...)

If this is the case, I would discourage you from overloading the scheduler APIs in such a way.

I'd have to fully understand what the intent here is before recommending a good path forward. Happy to meet and discuss.

Copy link
Contributor

@lgarg26 lgarg26 left a comment

Choose a reason for hiding this comment

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

Accepting to unblock ongoing intergration.

Summary:

Parameterize Scheduler
This not only more precisely expresses it's type (previously was unspecified generic variant of AppDryRunInfo[T] where T was unspecified; not it can be fully specified); This also allows us to inject PipelineDryRunInfo for schedulers that are pipeline schedulers

Reviewed By: lgarg26

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

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

@facebook-github-bot facebook-github-bot merged commit 9814e75 into meta-pytorch:main Mar 24, 2025
18 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.

4 participants