Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implementation of ReduceLROnPlateau scheduler #1754

Closed
ahmedo42 opened this issue Mar 7, 2021 · 3 comments 路 Fixed by #2449
Closed

Implementation of ReduceLROnPlateau scheduler #1754

ahmedo42 opened this issue Mar 7, 2021 · 3 comments 路 Fixed by #2449
Labels
enhancement

Comments

@ahmedo42
Copy link
Contributor

ahmedo42 commented Mar 7, 2021 •
edited

馃殌 Feature

While there might be some workarounds in using ReduceLROnPlateau with ignite's optimizer , it would be nicer to have a standalone feature like other schedulers in contrib.

Relevant issue and code sample.
sadra-barikbin added a commit to sadra-barikbin/ignite that referenced this issue Feb 3, 2022
@sadra-barikbin
Add ReduceLROnPlateauScheduler
ccf2364 
Closes pytorch#1754
@sadra-barikbin sadra-barikbin mentioned this issue Feb 3, 2022
Merged
3 tasks
@sadra-barikbin
Copy link
Collaborator

sadra-barikbin commented Feb 3, 2022 •
edited

Hi there,
I made this feature's PR. Is it OK? What else should I do?

@sdesrozis
Copy link
Contributor

@ahmedo42 It exists the class LRScheduler to wrap PyTorch schedulers. Out of curiosity, have you tried it? And if so, why doesn't it work on this case? Thanks a lot for the feedback!

@sadra-barikbin
Copy link
Collaborator

@sdesrozis LRScheduler does not support param_group_index and also It's not sufficient to just wrap a PyTorch's ReduceOnPlateau, but one should also feed its step function with a metric value.

vfdev-5 added a commit that referenced this issue Feb 20, 2022
@sadra-barikbin @vfdev-5
Add ReduceLROnPlateauScheduler feature (#2449)
ddd6527 
* Remove unnecessary code in BaseOutputHandler

Closes #2438

* Add ReduceLROnPlateauScheduler

Closes #1754

* Fix indentation issue

* Fix another indentation issue

* Fix PEP8 related issues

* Fix other PEP8 related issues

* Fix hopefully the last PEP8 related issue

* Fix hopefully the last PEP8 related issue

* Remove ReduceLROnPlateau's specific params and add link to it

Also fix bug in min_lr check

* Fix state_dict bug and add a test

* Update docs

* Add doctest and fix typo

Co-authored-by: vfdev <vfdev.5@gmail.com>
vfdev-5 added a commit that referenced this issue Apr 11, 2022
@sadra-barikbin @vfdev-5
Feature fixed weights hist handler issue 2328 (#2523)
7e05d91 
* Remove unnecessary code in BaseOutputHandler

Closes #2438

* Add ReduceLROnPlateauScheduler

Closes #1754

* Fix indentation issue

* Fix another indentation issue

* Fix PEP8 related issues

* Fix other PEP8 related issues

* Fix hopefully the last PEP8 related issue

* Fix hopefully the last PEP8 related issue

* Remove ReduceLROnPlateau's specific params and add link to it

Also fix bug in min_lr check

* Fix state_dict bug and add a test

* Update docs

* Add doctest and fix typo

* Add feature FixedWeightsHistHandler

Closes #2328

* Move FixedWeightsHistHandler's job to WeightsHistHandler

Closes #2328

* Enable whitelist to be callable

* autopep8 fix

* Refactor constructor

* Change whitelist to be List[str]

* Add whitelist callable type

* Fix bug in MNIST tensorboard example

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: sadra-barikbin <sadra-barikbin@users.noreply.github.com>
vfdev-5 added a commit that referenced this issue Apr 14, 2022
@sadra-barikbin @vfdev-5
Feature weights hist handler issue 2328 - Fix docstring (#2547)
581f5b4 
* Remove unnecessary code in BaseOutputHandler

Closes #2438

* Add ReduceLROnPlateauScheduler

Closes #1754

* Fix indentation issue

* Fix another indentation issue

* Fix PEP8 related issues

* Fix other PEP8 related issues

* Fix hopefully the last PEP8 related issue

* Fix hopefully the last PEP8 related issue

* Remove ReduceLROnPlateau's specific params and add link to it

Also fix bug in min_lr check

* Fix state_dict bug and add a test

* Update docs

* Add doctest and fix typo

* Add feature FixedWeightsHistHandler

Closes #2328

* Move FixedWeightsHistHandler's job to WeightsHistHandler

Closes #2328

* Enable whitelist to be callable

* autopep8 fix

* Refactor constructor

* Change whitelist to be List[str]

* Add whitelist callable type

* Fix bug in MNIST tensorboard example

* Fix docstring

* Update ignite/contrib/handlers/tensorboard_logger.py

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: sadra-barikbin <sadra-barikbin@users.noreply.github.com>
vfdev-5 added a commit that referenced this issue May 1, 2022
@sadra-barikbin @vfdev-5 @egaznep
Fix zero_grad place resulting in zero logs (#2555)
f14b184 
* Remove unnecessary code in BaseOutputHandler

Closes #2438

* Add ReduceLROnPlateauScheduler

Closes #1754

* Fix indentation issue

* Fix another indentation issue

* Fix PEP8 related issues

* Fix other PEP8 related issues

* Fix hopefully the last PEP8 related issue

* Fix hopefully the last PEP8 related issue

* Remove ReduceLROnPlateau's specific params and add link to it

Also fix bug in min_lr check

* Fix state_dict bug and add a test

* Update docs

* Fix gradients flushed at the end for 'supervised_train_step' (#2459)
I  have not changed the behavior of AMP, APEX, TPU as I don't have the means to test it.

* Add doctest and fix typo

* Fix gradient loggability for AMP, APEX and TPU (#2459)

* Fix zero_grad place in trainer step

Closes #2459 with help of PR #2470

* Improve tests and fix bug

* Remove redundant stmts after pytest parametrize

* Refactor tests

* autopep8 fix

* Improvement

* Fix bug

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: Unal Ege Gaznepoglu <egaznep@gmail.com>
Co-authored-by: sadra-barikbin <sadra-barikbin@users.noreply.github.com>
vfdev-5 added a commit that referenced this issue May 2, 2022
@sadra-barikbin @vfdev-5 @egaznep
Fix test on gpu (#2560)
a74b5b0 
* Remove unnecessary code in BaseOutputHandler

Closes #2438

* Add ReduceLROnPlateauScheduler

Closes #1754

* Fix indentation issue

* Fix another indentation issue

* Fix PEP8 related issues

* Fix other PEP8 related issues

* Fix hopefully the last PEP8 related issue

* Fix hopefully the last PEP8 related issue

* Remove ReduceLROnPlateau's specific params and add link to it

Also fix bug in min_lr check

* Fix state_dict bug and add a test

* Update docs

* Fix gradients flushed at the end for 'supervised_train_step' (#2459)
I  have not changed the behavior of AMP, APEX, TPU as I don't have the means to test it.

* Add doctest and fix typo

* Fix gradient loggability for AMP, APEX and TPU (#2459)

* Fix zero_grad place in trainer step

Closes #2459 with help of PR #2470

* Improve tests and fix bug

* Remove redundant stmts after pytest parametrize

* Refactor tests

* autopep8 fix

* Improvement

* Fix bug

* Fix test bugs in test_create_supervised

* Revert refactor

* Empty commit

* Fix pep

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: Unal Ege Gaznepoglu <egaznep@gmail.com>
Co-authored-by: sadra-barikbin <sadra-barikbin@users.noreply.github.com>
vfdev-5 added a commit that referenced this issue May 3, 2022
@sadra-barikbin @vfdev-5
Feature add whitelist parameter issue 2548 (#2550)
4f6b8b1 
* Remove unnecessary code in BaseOutputHandler

Closes #2438

* Add ReduceLROnPlateauScheduler

Closes #1754

* Fix indentation issue

* Fix another indentation issue

* Fix PEP8 related issues

* Fix other PEP8 related issues

* Fix hopefully the last PEP8 related issue

* Fix hopefully the last PEP8 related issue

* Remove ReduceLROnPlateau's specific params and add link to it

Also fix bug in min_lr check

* Fix state_dict bug and add a test

* Update docs

* Add doctest and fix typo

* Add whitelist param and refactor

Closes #2548

* Fix docstrings and a bug

* Change reduction parameter

* Fix zero_grad place in trainer step

Closes #2459 with help of PR #2470

* autopep8 fix

* Fix bugs

* Fix bugs in loggers

* Fix bug in test_create_supervised

* Change reduction type hint in base_logger

* Fix mypy error

* Fix bug causing missing clearml histograms

Co-authored-by: vfdev <vfdev.5@gmail.com>
Co-authored-by: sadra-barikbin <sadra-barikbin@users.noreply.github.com>
vfdev-5 added a commit that referenced this issue May 4, 2022
@sadra-barikbin @vfdev-5
Fix ssim run on cuda 2532 (#2564)
315b6b9 
* Remove unnecessary code in BaseOutputHandler

Closes #2438

* Add ReduceLROnPlateauScheduler

Closes #1754

* Fix indentation issue

* Fix another indentation issue

* Fix PEP8 related issues

* Fix other PEP8 related issues

* Fix hopefully the last PEP8 related issue

* Fix hopefully the last PEP8 related issue

* Remove ReduceLROnPlateau's specific params and add link to it

Also fix bug in min_lr check

* Fix state_dict bug and add a test

* Update docs

* Add doctest and fix typo

* fix bug to use on cuda

Co-authored-by: vfdev <vfdev.5@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add ReduceLROnPlateauScheduler feature sadra-barikbin/ignite
4 participants
@vfdev-5 @sdesrozis @sadra-barikbin @ahmedo42