Skip to content

Conversation

@sadra-barikbin
Copy link
Collaborator

@sadra-barikbin sadra-barikbin commented Feb 3, 2022

Fixes #1754

Description:
Add ReduceLROnPlateauScheduler which basically is a wrapper around torch.optim.lr_scheduler.ReduceLROnPlateau. It supports param_group_index and save_history params.
Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions bot added module: contrib Contrib module module: handlers Core Handlers module labels Feb 3, 2022
@vfdev-5 vfdev-5 requested a review from sdesrozis February 3, 2022 17:13
@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 3, 2022

@sadra-barikbin thanks for the PR, can you run the following to fix code format:

  • bash ./tests/run_code_style.sh install
  • bash ./tests/run_code_style.sh fmt

See https://github.com/pytorch/ignite/blob/master/CONTRIBUTING.md#formatting-code

@sadra-barikbin
Copy link
Collaborator Author

Hi,

@vfdev-5 Anything else should I do?

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 9, 2022

Hi @sadra-barikbin thanks for updates, I left few other comments. I checked how it works and now i understand that you are patching an original private method : self.scheduler._reduce_lr = self._reduce_lr to make things to work. Let's say it is OK.

@sadra-barikbin sadra-barikbin force-pushed the master branch 3 times, most recently from f159a56 to cdadf6c Compare February 10, 2022 03:32
Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sadra-barikbin thanks for the updates and sorry for delay !
Few nits about verbose and we can merge this PR.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 19, 2022

@sadra-barikbin can you resolve conflicts with master and address remaining comments. We can try to merge it then. Thanks a lot !

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update @sadra-barikbin !
Good to the PR.

@vfdev-5 vfdev-5 enabled auto-merge (squash) February 20, 2022 11:25
@vfdev-5 vfdev-5 merged commit ddd6527 into pytorch:master Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: contrib Contrib module module: handlers Core Handlers module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implementation of ReduceLROnPlateau scheduler

3 participants