clean warnings from tests and CI runs, and prepare for upgrade to PTL 1.8 #4830

nithinraok · 2022-08-29T00:53:43Z

Signed-off-by: nithinraok [email protected]

What does this PR do ?

Remove warnings from core and ASR code. NLP part needs more rework than expected.

Collection: Core, ASR

Changelog

weights_save_path has been removed
Some changes on ptl self.log sync_dist = True
torch.qr -> torch.linalg.qr
on_pretrain_routine_start -> on_fit_start
Update most metrics to have full_state_update = True variable
Pytest remove with_downloads marker warning
regex escape sequence and np float deprecation

Before your PR is "Ready for review"

Pre checks:

Make sure you read and followed Contributor guidelines
Did you write any new necessary tests?
Did you add or update any necessary documentation?
Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
- Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

New Feature
Bugfix
Documentation

lgtm-com · 2022-08-31T04:16:43Z

This pull request introduces 2 alerts when merging f23e11e into dd6e95e - view on LGTM.com

new alerts:

2 for Unused local variable

lgtm-com · 2022-08-31T05:18:34Z

This pull request introduces 2 alerts when merging 8c69a46 into dd6e95e - view on LGTM.com

new alerts:

2 for Unused local variable

lgtm-com · 2022-09-01T02:13:54Z

This pull request introduces 1 alert when merging 239b8e1 into 2ef4f35 - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2022-09-02T17:21:49Z

This pull request introduces 1 alert when merging c0482e5 into ea3c1b5 - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2022-09-06T18:03:39Z

This pull request introduces 1 alert when merging 36fbb43 into 1c16b96 - view on LGTM.com

new alerts:

1 for Unused import

nemo/collections/common/metrics/classification_accuracy.py

SeanNaren

LGTM

SeanNaren · 2022-09-12T20:31:55Z

(just in case, others don't merge @nithinraok just deciding if we wait for a branch cutoff)

titu1994

Overall looks good, just one question about why torch.long is removed ? It's necessary normally.

titu1994 · 2022-09-21T22:51:11Z

nemo/collections/asr/parts/submodules/ctc_greedy_decoding.py

@@ -31,7 +31,7 @@ def pack_hypotheses(hypotheses: List[rnnt_utils.Hypothesis], logitlen: torch.Ten
            logitlen_cpu = logitlen

    for idx, hyp in enumerate(hypotheses):  # type: rnnt_utils.Hypothesis
-        hyp.y_sequence = torch.tensor(hyp.y_sequence, dtype=torch.long)
+        hyp.y_sequence = torch.tensor(hyp.y_sequence,)


This should be long

titu1994 · 2022-09-21T22:54:18Z

tests/core/test_exp_manager.py


    def val_dataloader(self):
        dataset = OnesDataset(10)
-        return torch.utils.data.DataLoader(dataset, batch_size=2)
+        return torch.utils.data.DataLoader(dataset, batch_size=2, num_workers=8)


Are workers necessary for the test ?

Not mandatory, but lots of unnecessary warnings with workers=0.

Signed-off-by: nithinraok <[email protected]>

Signed-off-by: SeanNaren <[email protected]>

Signed-off-by: nithinraok <[email protected]>

titu1994

Looks great !

… 1.8 (#4830) * remove with_downloads marker warning from pytest Signed-off-by: nithinraok <[email protected]> * regex escape sequence and np float deprecation Signed-off-by: nithinraok <[email protected]> * improve speed of torchmetrics Signed-off-by: nithinraok <[email protected]> * fix python __int__ deprecation Signed-off-by: nithinraok <[email protected]> * multi binary accuracy Signed-off-by: nithinraok <[email protected]> * update topkaccuracy metric Signed-off-by: nithinraok <[email protected]> * trainer, text norm, qr -> linalg.qr Signed-off-by: nithinraok <[email protected]> * remove weights save path arg to trainer Signed-off-by: nithinraok <[email protected]> * ptl core warnings fix Signed-off-by: nithinraok <[email protected]> * on_pretrain_routine_start -> on_fit_start Signed-off-by: nithinraok <[email protected]> * add weights_save_path to deprecated args Signed-off-by: nithinraok <[email protected]> * style fix Signed-off-by: nithinraok <[email protected]> * Remove reference Signed-off-by: SeanNaren <[email protected]> * revert torch.long change Signed-off-by: nithinraok <[email protected]> Signed-off-by: nithinraok <[email protected]> Signed-off-by: SeanNaren <[email protected]> Co-authored-by: SeanNaren <[email protected]>

… 1.8 (#4830) * remove with_downloads marker warning from pytest Signed-off-by: nithinraok <[email protected]> * regex escape sequence and np float deprecation Signed-off-by: nithinraok <[email protected]> * improve speed of torchmetrics Signed-off-by: nithinraok <[email protected]> * fix python __int__ deprecation Signed-off-by: nithinraok <[email protected]> * multi binary accuracy Signed-off-by: nithinraok <[email protected]> * update topkaccuracy metric Signed-off-by: nithinraok <[email protected]> * trainer, text norm, qr -> linalg.qr Signed-off-by: nithinraok <[email protected]> * remove weights save path arg to trainer Signed-off-by: nithinraok <[email protected]> * ptl core warnings fix Signed-off-by: nithinraok <[email protected]> * on_pretrain_routine_start -> on_fit_start Signed-off-by: nithinraok <[email protected]> * add weights_save_path to deprecated args Signed-off-by: nithinraok <[email protected]> * style fix Signed-off-by: nithinraok <[email protected]> * Remove reference Signed-off-by: SeanNaren <[email protected]> * revert torch.long change Signed-off-by: nithinraok <[email protected]> Signed-off-by: nithinraok <[email protected]> Signed-off-by: SeanNaren <[email protected]> Co-authored-by: SeanNaren <[email protected]> Signed-off-by: George Zelenfroynd <[email protected]>

… 1.8 (NVIDIA#4830) * remove with_downloads marker warning from pytest Signed-off-by: nithinraok <[email protected]> * regex escape sequence and np float deprecation Signed-off-by: nithinraok <[email protected]> * improve speed of torchmetrics Signed-off-by: nithinraok <[email protected]> * fix python __int__ deprecation Signed-off-by: nithinraok <[email protected]> * multi binary accuracy Signed-off-by: nithinraok <[email protected]> * update topkaccuracy metric Signed-off-by: nithinraok <[email protected]> * trainer, text norm, qr -> linalg.qr Signed-off-by: nithinraok <[email protected]> * remove weights save path arg to trainer Signed-off-by: nithinraok <[email protected]> * ptl core warnings fix Signed-off-by: nithinraok <[email protected]> * on_pretrain_routine_start -> on_fit_start Signed-off-by: nithinraok <[email protected]> * add weights_save_path to deprecated args Signed-off-by: nithinraok <[email protected]> * style fix Signed-off-by: nithinraok <[email protected]> * Remove reference Signed-off-by: SeanNaren <[email protected]> * revert torch.long change Signed-off-by: nithinraok <[email protected]> Signed-off-by: nithinraok <[email protected]> Signed-off-by: SeanNaren <[email protected]> Co-authored-by: SeanNaren <[email protected]> Signed-off-by: Matvei Novikov <[email protected]>

… 1.8 (NVIDIA#4830) * remove with_downloads marker warning from pytest Signed-off-by: nithinraok <[email protected]> * regex escape sequence and np float deprecation Signed-off-by: nithinraok <[email protected]> * improve speed of torchmetrics Signed-off-by: nithinraok <[email protected]> * fix python __int__ deprecation Signed-off-by: nithinraok <[email protected]> * multi binary accuracy Signed-off-by: nithinraok <[email protected]> * update topkaccuracy metric Signed-off-by: nithinraok <[email protected]> * trainer, text norm, qr -> linalg.qr Signed-off-by: nithinraok <[email protected]> * remove weights save path arg to trainer Signed-off-by: nithinraok <[email protected]> * ptl core warnings fix Signed-off-by: nithinraok <[email protected]> * on_pretrain_routine_start -> on_fit_start Signed-off-by: nithinraok <[email protected]> * add weights_save_path to deprecated args Signed-off-by: nithinraok <[email protected]> * style fix Signed-off-by: nithinraok <[email protected]> * Remove reference Signed-off-by: SeanNaren <[email protected]> * revert torch.long change Signed-off-by: nithinraok <[email protected]> Signed-off-by: nithinraok <[email protected]> Signed-off-by: SeanNaren <[email protected]> Co-authored-by: SeanNaren <[email protected]>

… 1.8 (NVIDIA#4830) * remove with_downloads marker warning from pytest Signed-off-by: nithinraok <[email protected]> * regex escape sequence and np float deprecation Signed-off-by: nithinraok <[email protected]> * improve speed of torchmetrics Signed-off-by: nithinraok <[email protected]> * fix python __int__ deprecation Signed-off-by: nithinraok <[email protected]> * multi binary accuracy Signed-off-by: nithinraok <[email protected]> * update topkaccuracy metric Signed-off-by: nithinraok <[email protected]> * trainer, text norm, qr -> linalg.qr Signed-off-by: nithinraok <[email protected]> * remove weights save path arg to trainer Signed-off-by: nithinraok <[email protected]> * ptl core warnings fix Signed-off-by: nithinraok <[email protected]> * on_pretrain_routine_start -> on_fit_start Signed-off-by: nithinraok <[email protected]> * add weights_save_path to deprecated args Signed-off-by: nithinraok <[email protected]> * style fix Signed-off-by: nithinraok <[email protected]> * Remove reference Signed-off-by: SeanNaren <[email protected]> * revert torch.long change Signed-off-by: nithinraok <[email protected]> Signed-off-by: nithinraok <[email protected]> Signed-off-by: SeanNaren <[email protected]> Co-authored-by: SeanNaren <[email protected]> Signed-off-by: Matvei Novikov <[email protected]>

… 1.8 (NVIDIA#4830) * remove with_downloads marker warning from pytest Signed-off-by: nithinraok <[email protected]> * regex escape sequence and np float deprecation Signed-off-by: nithinraok <[email protected]> * improve speed of torchmetrics Signed-off-by: nithinraok <[email protected]> * fix python __int__ deprecation Signed-off-by: nithinraok <[email protected]> * multi binary accuracy Signed-off-by: nithinraok <[email protected]> * update topkaccuracy metric Signed-off-by: nithinraok <[email protected]> * trainer, text norm, qr -> linalg.qr Signed-off-by: nithinraok <[email protected]> * remove weights save path arg to trainer Signed-off-by: nithinraok <[email protected]> * ptl core warnings fix Signed-off-by: nithinraok <[email protected]> * on_pretrain_routine_start -> on_fit_start Signed-off-by: nithinraok <[email protected]> * add weights_save_path to deprecated args Signed-off-by: nithinraok <[email protected]> * style fix Signed-off-by: nithinraok <[email protected]> * Remove reference Signed-off-by: SeanNaren <[email protected]> * revert torch.long change Signed-off-by: nithinraok <[email protected]> Signed-off-by: nithinraok <[email protected]> Signed-off-by: SeanNaren <[email protected]> Co-authored-by: SeanNaren <[email protected]>

… 1.8 (NVIDIA#4830) * remove with_downloads marker warning from pytest Signed-off-by: nithinraok <[email protected]> * regex escape sequence and np float deprecation Signed-off-by: nithinraok <[email protected]> * improve speed of torchmetrics Signed-off-by: nithinraok <[email protected]> * fix python __int__ deprecation Signed-off-by: nithinraok <[email protected]> * multi binary accuracy Signed-off-by: nithinraok <[email protected]> * update topkaccuracy metric Signed-off-by: nithinraok <[email protected]> * trainer, text norm, qr -> linalg.qr Signed-off-by: nithinraok <[email protected]> * remove weights save path arg to trainer Signed-off-by: nithinraok <[email protected]> * ptl core warnings fix Signed-off-by: nithinraok <[email protected]> * on_pretrain_routine_start -> on_fit_start Signed-off-by: nithinraok <[email protected]> * add weights_save_path to deprecated args Signed-off-by: nithinraok <[email protected]> * style fix Signed-off-by: nithinraok <[email protected]> * Remove reference Signed-off-by: SeanNaren <[email protected]> * revert torch.long change Signed-off-by: nithinraok <[email protected]> Signed-off-by: nithinraok <[email protected]> Signed-off-by: SeanNaren <[email protected]> Co-authored-by: SeanNaren <[email protected]> Signed-off-by: Hainan Xu <[email protected]>

nithinraok force-pushed the clean_warnings_and_upgrade_ptl branch from 36a6827 to f23e11e Compare August 31, 2022 04:01

nithinraok force-pushed the clean_warnings_and_upgrade_ptl branch from 22f0406 to 239b8e1 Compare September 1, 2022 01:45

nithinraok force-pushed the clean_warnings_and_upgrade_ptl branch from 239b8e1 to c0482e5 Compare September 2, 2022 17:10

nithinraok force-pushed the clean_warnings_and_upgrade_ptl branch from c0482e5 to 36fbb43 Compare September 6, 2022 17:50

nithinraok marked this pull request as ready for review September 6, 2022 17:57

nithinraok requested review from titu1994, SeanNaren and ericharper September 6, 2022 17:57

nithinraok force-pushed the clean_warnings_and_upgrade_ptl branch from 4747902 to 3c6403c Compare September 7, 2022 21:05

nithinraok changed the title ~~remove with_downloads marker warning from pytest~~ clean warnings from tests and CI runs, and prepare for upgrade to PTL 1.8 Sep 7, 2022

nithinraok force-pushed the clean_warnings_and_upgrade_ptl branch from 76b0ad2 to 4a169f5 Compare September 12, 2022 17:07

SeanNaren reviewed Sep 12, 2022

View reviewed changes

nemo/collections/common/metrics/classification_accuracy.py Outdated Show resolved Hide resolved

SeanNaren previously approved these changes Sep 12, 2022

View reviewed changes

SeanNaren self-requested a review September 12, 2022 21:02

titu1994 previously approved these changes Sep 21, 2022

View reviewed changes

nithinraok dismissed stale reviews from titu1994 and SeanNaren via 606ecc6 September 21, 2022 23:09

nithinraok force-pushed the clean_warnings_and_upgrade_ptl branch from bee1c15 to 606ecc6 Compare September 21, 2022 23:09

nithinraok added 6 commits September 23, 2022 17:50

remove with_downloads marker warning from pytest

f9ca979

Signed-off-by: nithinraok <[email protected]>

regex escape sequence and np float deprecation

ab495e5

Signed-off-by: nithinraok <[email protected]>

improve speed of torchmetrics

c6846aa

Signed-off-by: nithinraok <[email protected]>

fix python __int__ deprecation

1ce7fcf

Signed-off-by: nithinraok <[email protected]>

multi binary accuracy

dcd3c7d

Signed-off-by: nithinraok <[email protected]>

update topkaccuracy metric

b9d455f

Signed-off-by: nithinraok <[email protected]>

nithinraok and others added 8 commits September 23, 2022 17:50

trainer, text norm, qr -> linalg.qr

907b11d

Signed-off-by: nithinraok <[email protected]>

remove weights save path arg to trainer

72e8306

Signed-off-by: nithinraok <[email protected]>

ptl core warnings fix

2239899

Signed-off-by: nithinraok <[email protected]>

on_pretrain_routine_start -> on_fit_start

db08728

Signed-off-by: nithinraok <[email protected]>

add weights_save_path to deprecated args

65b33fb

Signed-off-by: nithinraok <[email protected]>

style fix

e965a6a

Signed-off-by: nithinraok <[email protected]>

Remove reference

8e51508

Signed-off-by: SeanNaren <[email protected]>

revert torch.long change

cefad2f

Signed-off-by: nithinraok <[email protected]>

nithinraok force-pushed the clean_warnings_and_upgrade_ptl branch from 606ecc6 to cefad2f Compare September 24, 2022 01:03

Merge branch 'main' into clean_warnings_and_upgrade_ptl

b6938fe

nithinraok requested review from titu1994 and removed request for ericharper September 26, 2022 01:33

titu1994 approved these changes Sep 26, 2022

View reviewed changes

titu1994 merged commit e3ac280 into main Sep 26, 2022

nithinraok deleted the clean_warnings_and_upgrade_ptl branch September 26, 2022 01:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clean warnings from tests and CI runs, and prepare for upgrade to PTL 1.8 #4830

clean warnings from tests and CI runs, and prepare for upgrade to PTL 1.8 #4830

nithinraok commented Aug 29, 2022 •

edited

Loading

lgtm-com bot commented Aug 31, 2022

lgtm-com bot commented Aug 31, 2022

lgtm-com bot commented Sep 1, 2022

lgtm-com bot commented Sep 2, 2022

lgtm-com bot commented Sep 6, 2022

SeanNaren left a comment

SeanNaren commented Sep 12, 2022

titu1994 left a comment

titu1994 Sep 21, 2022

titu1994 Sep 21, 2022

nithinraok Sep 21, 2022

titu1994 left a comment

clean warnings from tests and CI runs, and prepare for upgrade to PTL 1.8 #4830

clean warnings from tests and CI runs, and prepare for upgrade to PTL 1.8 #4830

Conversation

nithinraok commented Aug 29, 2022 • edited Loading

What does this PR do ?

Changelog

Before your PR is "Ready for review"

lgtm-com bot commented Aug 31, 2022

lgtm-com bot commented Aug 31, 2022

lgtm-com bot commented Sep 1, 2022

lgtm-com bot commented Sep 2, 2022

lgtm-com bot commented Sep 6, 2022

SeanNaren left a comment

Choose a reason for hiding this comment

SeanNaren commented Sep 12, 2022

titu1994 left a comment

Choose a reason for hiding this comment

titu1994 Sep 21, 2022

Choose a reason for hiding this comment

titu1994 Sep 21, 2022

Choose a reason for hiding this comment

nithinraok Sep 21, 2022

Choose a reason for hiding this comment

titu1994 left a comment

Choose a reason for hiding this comment

nithinraok commented Aug 29, 2022 •

edited

Loading