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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Seemingly unnecessary dictionary comprehension in contrib's BaseOutputHandler #2438

Closed
sadra-barikbin opened this issue Jan 22, 2022 · 2 comments · Fixed by #2439
Closed

Seemingly unnecessary dictionary comprehension in contrib's BaseOutputHandler #2438

sadra-barikbin opened this issue Jan 22, 2022 · 2 comments · Fixed by #2439

Comments

@sadra-barikbin
Copy link
Collaborator

Hi,
output_dict is made sure to be a dictionary just above the line below.

ignite/ignite/contrib/handlers/base_logger.py

Line 108 in 21a2ccc

metrics_state_attrs.update({name: value for name, value in output_dict.items()})

So this line could be replaced with:

metrics_state_attrs.update(output_dict)
@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jan 22, 2022

Thanks for pointing out that, @sadra-barikbin !
Would you like to send a PR to fix that ?

@sadra-barikbin
Copy link
Collaborator Author

Yes, sure!

sadra-barikbin added a commit to sadra-barikbin/ignite that referenced this issue Jan 22, 2022
@sadra-barikbin
Remove unnecessary code in BaseOutputHandler
3484a59 
Closes pytorch#2438
@sadra-barikbin sadra-barikbin mentioned this issue Jan 22, 2022
Merged
3 tasks
vfdev-5 pushed a commit that referenced this issue Jan 22, 2022
@sadra-barikbin
Remove unnecessary code in BaseOutputHandler (#2439)
e2d9047 
Closes #2438
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
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove unnecessary code in BaseOutputHandler sadra-barikbin/ignite
2 participants
@vfdev-5 @sadra-barikbin