-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add deprecation path for renamed training type plugins #11227
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
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
992b6c7 to
903e6fd
Compare
| pytest.param(DeepSpeedPlugin, marks=RunIf(deepspeed=True)), | ||
| DataParallelPlugin, | ||
| DDPFullyShardedPlugin, | ||
| pytest.param(IPUPlugin, marks=RunIf(ipu=True)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't run at the moment because we only run ipu tests on the ipu test file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think #11170 will land? I can drop the test case completely, move it to the ipu test file, or keep as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. I would move the test to test_ipu.py and add a comment here referencing it.
650a04e to
4cd25b2
Compare
for more information, see https://pre-commit.ci
tchaton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
Co-authored-by: Kaushik B <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
What does this PR do?
Part of #10549
Adds a deprecation path for the training type plugins that we renamed in #11226.
Example: In 1.6 this usage will be deprecated
and should be replaced with
TODO:
Does your PR introduce any breaking changes? If yes, please list them.
No, but after v1.8, when the old plugin aliases are removed, all imports will break.
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
I made sure I had fun coding 🙃
Part of #1 (it's a lie, this is just here to avoid noisy GitHub bot)
cc @tchaton @rohitgr7 @Borda