-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix issues with PL 1.8 #5353
Fix issues with PL 1.8 #5353
Conversation
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
# Conflicts: # nemo/utils/exp_manager.py
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
This PR should hopefully unblock the move to 1.8: Lightning-AI/pytorch-lightning#15737 (review) We'll just wait till it gets into a minor release version (which hopefully would be tomorrow). |
# Conflicts: # nemo/core/classes/exportable.py
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
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.
Overall, incredible work @SeanNaren ! Minor comments and needs one other reviewer since it's a core change
@@ -128,7 +129,7 @@ def _export( | |||
# Set module mode | |||
with torch.onnx.select_model_mode_for_export( | |||
self, training | |||
), torch.inference_mode(), torch.no_grad(), torch.jit.optimized_execution(True): | |||
), torch.inference_mode(), torch.no_grad(), torch.jit.optimized_execution(True), _jit_is_scripting(): |
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.
Hmm we don't use lightning during export, is this method safe ?
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.
Would the tests have confirmed if this is fine? The underlying code is simple to what is happening:
In Lightning:
@contextmanager
def _jit_is_scripting() -> Generator:
"""Workaround for https://github.com/pytorch/pytorch/issues/67146."""
LightningModule._jit_is_scripting = True
try:
yield
finally:
LightningModule._jit_is_scripting = False
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.
Hmm i dont think it would be caught in tests, lets keep it for now
Signed-off-by: SeanNaren <snarenthiran@nvidia.com>
This pull request fixes 1 alert when merging d614378 into 2430d0f - view on LGTM.com fixed alerts:
Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. It looks like GitHub code scanning with CodeQL is already set up for this repo, so no further action is needed 🚀. For more information, please check out our post on the GitHub blog. |
* Fix issues with PL 1.8 Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Set scripting variable Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix missing arg Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Cleanup list Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix reference Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Try to fix hanging EMA test Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Missing \ Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Add strategy Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * See if setting the chdir fixes the hanging DDP test Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * See if removing the subdir setter fixes the issue Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Remove checks Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Try [0,1] for devices Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Add code back Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Remove space Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Update requirements Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Swap import path Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Update references Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix deprecated variables Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix missing var Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix var Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Revert changes Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Address review Signed-off-by: SeanNaren <snarenthiran@nvidia.com> Signed-off-by: SeanNaren <snarenthiran@nvidia.com> Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com> Signed-off-by: Hainan Xu <hainanx@nvidia.com>
* Fix issues with PL 1.8 Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Set scripting variable Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix missing arg Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Cleanup list Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix reference Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Try to fix hanging EMA test Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Missing \ Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Add strategy Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * See if setting the chdir fixes the hanging DDP test Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * See if removing the subdir setter fixes the issue Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Remove checks Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Try [0,1] for devices Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Add code back Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Remove space Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Update requirements Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Swap import path Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Update references Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix deprecated variables Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix missing var Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix var Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Revert changes Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Address review Signed-off-by: SeanNaren <snarenthiran@nvidia.com> Signed-off-by: SeanNaren <snarenthiran@nvidia.com> Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com> Signed-off-by: Hainan Xu <hainanx@nvidia.com>
* Fix issues with PL 1.8 Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Set scripting variable Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix missing arg Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Cleanup list Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix reference Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Try to fix hanging EMA test Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Missing \ Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Add strategy Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * See if setting the chdir fixes the hanging DDP test Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * See if removing the subdir setter fixes the issue Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Remove checks Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Try [0,1] for devices Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Add code back Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Remove space Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Update requirements Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Swap import path Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Update references Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix deprecated variables Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix missing var Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix var Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Revert changes Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Address review Signed-off-by: SeanNaren <snarenthiran@nvidia.com> Signed-off-by: SeanNaren <snarenthiran@nvidia.com> Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com> Signed-off-by: Hainan Xu <hainanx@nvidia.com>
* Fix issues with PL 1.8 Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Set scripting variable Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix missing arg Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Cleanup list Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix reference Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Try to fix hanging EMA test Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Missing \ Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Add strategy Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * See if setting the chdir fixes the hanging DDP test Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * See if removing the subdir setter fixes the issue Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Remove checks Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Try [0,1] for devices Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Add code back Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Remove space Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Update requirements Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Swap import path Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Update references Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix deprecated variables Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix missing var Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Fix var Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Revert changes Signed-off-by: SeanNaren <snarenthiran@nvidia.com> * Address review Signed-off-by: SeanNaren <snarenthiran@nvidia.com> Signed-off-by: SeanNaren <snarenthiran@nvidia.com> Co-authored-by: Oleksii Kuchaiev <okuchaiev@users.noreply.github.com> Signed-off-by: andrusenkoau <andrusenkoau@gmail.com>
What does this PR do ?
Attempts to fix all issues after many deprecations/lightning lite becoming the backbone for a lot of the underlying pytorch lightning code.
cc @titu1994
Before your PR is "Ready for review"
Pre checks:
PR Type: