Changed LearningRateLogger to LearningRateMonitor

[rohitgr7]

What does this PR do?

Changed LearningRateLogger to LearningRateMonitor. Discussion: #3008 (comment)

Before submitting

• Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
• Did you make sure to update the documentation with your changes?
• Did you write any new necessary tests?
• Did you verify new and existing tests pass locally with your changes?
• If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

[codecov]

Codecov Report

Merging #3251 into master will increase coverage by 0%.
The diff coverage is 97%.

@@          Coverage Diff           @@
##           master   #3251   +/-   ##
======================================
  Coverage      90%     90%           
======================================
  Files          90      91    +1     
  Lines        8138    8143    +5     
======================================
+ Hits         7349    7355    +6     
+ Misses        789     788    -1     

[awaelchli]

@rohitgr7 I think we need to make sure that LearningRateLogger can still be used, so maybe subclass

class LearningRateLogger(LearningRateMonitor):
    def __init__(self, *args, **kwargs)
        # print a deprecation warning here
        rank_zero_warn(...)
        super().__init__(*args, **kwargs)

[rohitgr7]

Also, should we consider the row_log_interval here, just like with usual logs?

[awaelchli]

LGTM, maybe ask @Borda to check deprecation stuff :)

[awaelchli]

Also, should we consider the row_log_interval here, just like with usual logs?

probably, yes, in new PR?

[Borda]

LGTM, maybe ask @Borda to check deprecation stuff :)

yeah, this will be quite tricky, there are a few things you need to do:

• keep the original file and reimport from new.package.path import NewClass as OldClass
• add deprecation warning to the old file
• add a test that with calling the old class the deprecation warning is raised

[Borda]

need to maintain back compatibility... see #3251 (comment)

[rohitgr7]

Also, should we consider the row_log_interval here, just like with usual logs?

probably, yes, in new PR?

ok, will add for both LearningRateMonitor and GPUStatsMonitor in a separate one.

[rohitgr7]

@Borda can you check if the depreciation is done correctly as you asked :) Thanks!

[SkafteNicki]

LGTM

[rohitgr7]

@Borda can you review this? Thanks

[mergify]

This pull request is now in conflict... :(

[mergify]

Great job! =)