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

typing scheduler.py [1/3]: Add NodeSchedulerNode #126609

Closed
wants to merge 5 commits into from

Conversation

aorenste
Copy link
Contributor

@aorenste aorenste commented May 18, 2024

Split off to make reviewing easier.

BaseSchedulerNode really represents two types of nodes - ones where the node field is valid (most of them) and ones where the node field isn't (ones derived from FusedSchedulerNode). In the FusedSchedulerNode case we also don't want to run BaseSchedulerNode's __init__. FusedSchedulerNode also overrides a bunch of methods to throw NotImplementedError.

Add a new class, NodeSchedulerNode to represent nodes where node is valid and move the methods that FusedSchedulerNode doesn't care about onto it.

No actual code changes - just moving methods around.

Stack from ghstack (oldest at bottom):

[ghstack-poisoned]
Copy link

pytorch-bot bot commented May 18, 2024

🔗 Helpful Links

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

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

✅ You can merge normally! (7 Unrelated Failures)

As of commit 95ea0a7 with merge base a44d0cf (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

BROKEN TRUNK - The following jobs failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

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

@aorenste aorenste changed the title Add NodeSchedulerNode typing scheduler.py [2/4]: Add NodeSchedulerNode May 18, 2024
Split off to make reviewing easier.

`BaseSchedulerNode` really represents two types of nodes - ones where the `node` field is valid (most of them) and ones where the `node` field isn't (ones derived from `FusedSchedulerNode`).  In the `FusedSchedulerNode` case we also don't want to run `BaseSchedulerNode`'s `__init__`.  `FusedSchedulerNode` also overrides a bunch of methods to throw `NotImplementedError`.

Add a new class, `NodeSchedulerNode` to represent nodes where `node` is valid and move the methods that `FusedSchedulerNode` doesn't care about onto it.

No actual code changes - just moving methods around.




[ghstack-poisoned]
Split off to make reviewing easier.

`BaseSchedulerNode` really represents two types of nodes - ones where the `node` field is valid (most of them) and ones where the `node` field isn't (ones derived from `FusedSchedulerNode`).  In the `FusedSchedulerNode` case we also don't want to run `BaseSchedulerNode`'s `__init__`.  `FusedSchedulerNode` also overrides a bunch of methods to throw `NotImplementedError`.

Add a new class, `NodeSchedulerNode` to represent nodes where `node` is valid and move the methods that `FusedSchedulerNode` doesn't care about onto it.

No actual code changes - just moving methods around.




[ghstack-poisoned]
Split off to make reviewing easier.

`BaseSchedulerNode` really represents two types of nodes - ones where the `node` field is valid (most of them) and ones where the `node` field isn't (ones derived from `FusedSchedulerNode`).  In the `FusedSchedulerNode` case we also don't want to run `BaseSchedulerNode`'s `__init__`.  `FusedSchedulerNode` also overrides a bunch of methods to throw `NotImplementedError`.

Add a new class, `NodeSchedulerNode` to represent nodes where `node` is valid and move the methods that `FusedSchedulerNode` doesn't care about onto it.

No actual code changes - just moving methods around.




[ghstack-poisoned]
@aorenste aorenste changed the title typing scheduler.py [2/4]: Add NodeSchedulerNode typing scheduler.py [1/3]: Add NodeSchedulerNode May 18, 2024
Split off to make reviewing easier.

`BaseSchedulerNode` really represents two types of nodes - ones where the `node` field is valid (most of them) and ones where the `node` field isn't (ones derived from `FusedSchedulerNode`).  In the `FusedSchedulerNode` case we also don't want to run `BaseSchedulerNode`'s `__init__`.  `FusedSchedulerNode` also overrides a bunch of methods to throw `NotImplementedError`.

Add a new class, `NodeSchedulerNode` to represent nodes where `node` is valid and move the methods that `FusedSchedulerNode` doesn't care about onto it.

No actual code changes - just moving methods around.




[ghstack-poisoned]
@aorenste aorenste closed this May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant