added TimeLimit handler with its test and doc by ahmedo42 · Pull Request #1611 · pytorch/ignite

ahmedo42 · 2021-02-04T17:26:18Z

closes #936

Description: added TimeLimit handler with its test and doc

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)

ahmedo42 · 2021-02-04T18:49:17Z

I realized that I didn't build docs correctly and they are incomplete , I'm closing this while I fix it

vfdev-5 · 2021-02-04T19:17:16Z

Thanks for the PR @ahmedo42 !
No worries about having the PR open and as a draft. Let me reopen it and set as draft.

I'll review it later today or tomorrow..

vfdev-5

Thanks for the updates @ahmedo42 ! Looks good ! Just few comments about logging and testing.

vfdev-5 · 2021-02-05T12:06:13Z

ignite/handlers/time_limit.py

+
+from ignite.engine import Engine
+
+log = logging.getLogger(__name__)


Currently, we create an instance of the logger inside the class, for example:

ignite/ignite/handlers/early_stopping.py

Line 79 in c25ce51

self.logger = logging.getLogger(__name__ + "." + self.__class__.__name__)

Let's do it like that here too.

Please, remove log.

ignite/handlers/time_limit.py

tests/ignite/handlers/test_time_limit.py

vfdev-5

Thanks for the updates!
Few other comments to adress.

vfdev-5 · 2021-02-05T15:57:08Z

ignite/handlers/time_limit.py

+
+from ignite.engine import Engine
+
+log = logging.getLogger(__name__)


Please, remove log.

ignite/handlers/time_limit.py

tests/ignite/handlers/test_time_limit.py

ignite/handlers/time_limit.py

tests/ignite/handlers/test_time_limit.py

ignite/handlers/time_limit.py

vfdev-5 · 2021-02-05T17:00:56Z

@ahmedo42 it remains default limit_sec to fix and remove global log

ahmedo42 · 2021-02-05T17:30:36Z

@vfdev-5 Thanks for your patience and comments

vfdev-5

LGTM! Thanks @ahmedo42 !

ignite/handlers/time_limit.py

tests/ignite/handlers/test_time_limit.py

sdesrozis · 2021-02-05T21:33:50Z

@ahmedo42 few comments. Thank you again for your help.

…into TimeLimit-handler

sdesrozis

LGTM ! Thank you !

* added TimeLimit handler with its test and doc * fixed documentation * fixed docstring and formatting * flake8 fix trailing whitespace :) * modified class logger , default value and tests * changed rounding to nearest integer * tests refactored , docs modified * fixed default value , removed global logger * fixing formatting * Added versionadded * added test for engine termination Co-authored-by: vfdev <[email protected]>

@deprecated

* Recall/Precision metrics for ddp : average == false and multilabel == true * For v0.4.3 - Add more versionadded, versionchanged tags - Change v0.5… (#1612) * For v0.4.3 - Add more versionadded, versionchanged tags - Change v0.5.0 to v0.4.3 * Update ignite/contrib/metrics/regression/canberra_metric.py Co-authored-by: vfdev <[email protected]> * Update ignite/contrib/metrics/regression/manhattan_distance.py Co-authored-by: vfdev <[email protected]> * Update ignite/contrib/metrics/regression/r2_score.py Co-authored-by: vfdev <[email protected]> * Update ignite/handlers/checkpoint.py Co-authored-by: vfdev <[email protected]> * address PR comments Co-authored-by: vfdev <[email protected]> * added TimeLimit handler with its test and doc (#1611) * added TimeLimit handler with its test and doc * fixed documentation * fixed docstring and formatting * flake8 fix trailing whitespace :) * modified class logger , default value and tests * changed rounding to nearest integer * tests refactored , docs modified * fixed default value , removed global logger * fixing formatting * Added versionadded * added test for engine termination Co-authored-by: vfdev <[email protected]> * Update handlers to use setup_logger (#1617) * Fixes #1614 - Updated handlers EarlyStopping and TerminateOnNan - Replaced `logging.getLogger` with `setup_logger` in the mentioned handlers * Updated `TimeLimit` handler. Replaced use of `logger.getLogger` with `setup_logger` from `ignite.utils` Co-authored-by: Pradyumna Rahul K <[email protected]> Co-authored-by: Sylvain Desroziers <[email protected]> * Managing Deprecation using decorators (#1585) * Starter code for managing deprecation * Make functions deprecated using the `@deprecated` decorator * Add arguments to the @deprecated decorator to customize it for each function * Improve `@deprecated` decorator and add tests * Replaced the `raise` keyword with added `warnings` * Added tests several possibilities of the decorator usage * Removing the test deprecation to check tests * Add static typing, fix mypy errors * Make `@deprecated` to raise Exceptions or Warning * The `@deprecated` decorator will now always emit warning unless explicitly asked to raise an Exception * Fix mypy errors * Fix mypy errors (hopefully) * Fix the test `test_deprecated_setup_any_logging` * Change the test to work with the `@deprecated` decorator * Change to snake_case, handle mypy ignores * Improve Type Annotations * Update common.py * For v0.4.3 - Add more versionadded, versionchanged tags - Change v0.5… (#1612) * For v0.4.3 - Add more versionadded, versionchanged tags - Change v0.5.0 to v0.4.3 * Update ignite/contrib/metrics/regression/canberra_metric.py Co-authored-by: vfdev <[email protected]> * Update ignite/contrib/metrics/regression/manhattan_distance.py Co-authored-by: vfdev <[email protected]> * Update ignite/contrib/metrics/regression/r2_score.py Co-authored-by: vfdev <[email protected]> * Update ignite/handlers/checkpoint.py Co-authored-by: vfdev <[email protected]> * address PR comments Co-authored-by: vfdev <[email protected]> * `version` -> version Co-authored-by: vfdev <[email protected]> Co-authored-by: François COKELAER <[email protected]> Co-authored-by: Sylvain Desroziers <[email protected]> * Create documentation.md * Distributed tests on Windows should be skipped until fixed. (#1620) * modified CONTRIBUTING.md * bash instead of sh * Added Checkpoint.get_default_score_fn (#1621) * Added Checkpoint.get_default_score_fn to simplify best_model_handler creation * Added score_sign argument * Updated docs * Update about.rst * Update pre-commit hooks and CONTRIBUTING.md (#1622) * Change pre-commit config and CONTRIBUTING.md - Update hook versions - Remove seed-isort-config - Add black profile to isort * Fix files based on new pre-commit config * Add meaningful exclusions to prettier - Also update actions workflow files to match local pre-commit * added requirements.txt and updated readme.md (#1624) * added requirements.txt and updated readme.md * Update examples/contrib/cifar10/README.md Co-authored-by: vfdev <[email protected]> * Update examples/contrib/cifar10/requirements.txt Co-authored-by: vfdev <[email protected]> Co-authored-by: vfdev <[email protected]> * Replace relative paths with raw.githubusercontent (#1629) * Updated cifar10 example (#1632) * Updates for cifar10 example * Updates for cifar10 example * More updates * Updated code * Fixed code-formatting * Fixed failling CI and typos for cifar10 examples (#1633) * Updates for cifar10 example * Updates for cifar10 example * More updates * Updated code * Fixed code-formatting * Fixed typo and failing CI * Fixed hvd spawn fail and better synced qat code * Removed temporary hack to install pth 1.7.1 (#1638) - updated default pth image for gpu tests - updated TORCH_CUDA_ARCH_LIST - fixed /merge -> /head in trigger ci pipeline * [docker] Pillow -> Pillow-SIMD (#1509) (#1639) * [docker] Pillow -> Pillow-SIMD (#1509) * [docker] Pillow -> Pillow-SIMD * replace pillow with pillow-simd in base docker files * chore(docker): apt-get autoremove after pillow-simd installation * apt-get install at once, autoremove g++ * install g++ in pillow installation layer Co-authored-by: Sylvain Desroziers <[email protected]> * Fix g++ install issue Co-authored-by: Jeff Yang <[email protected]> Co-authored-by: Sylvain Desroziers <[email protected]> * Fix multinode tests script (#1631) * fix run_multinode_tests_in_docker.sh : run tests with docker python version * add missing modules * build an image with test env and add 'nnodes' 'nproc_per_node' 'gpu' as parameters * #1615 : change nproc_per_node default to 4 * #1615 : fix for gpu enabled tests / container rm step at the end of the script * add xfail decorator for tests/ignite/engine/test_deterministic.py::test_multinode_distrib_cpu * fix script gpu_options * add default tol=1e-6 for _test_distrib_compute_on_criterion * fix for "RuntimeError: trying to initialize the default process group twice!" * tolerance for test_multinode_distrib_cpu case only * fix assert None error * autopep8 fix Co-authored-by: vfdev <[email protected]> Co-authored-by: Sylvain Desroziers <[email protected]> Co-authored-by: fco-dv <[email protected]> * remove warning for average=False and is_multilabel=True * update docstring and {precision, recall} tests according to test_multilabel_input_NCHW Co-authored-by: vfdev <[email protected]> Co-authored-by: Ahmed Omar <[email protected]> Co-authored-by: Pradyumna Rahul <[email protected]> Co-authored-by: Pradyumna Rahul K <[email protected]> Co-authored-by: Sylvain Desroziers <[email protected]> Co-authored-by: Devanshu Shah <[email protected]> Co-authored-by: Debojyoti Chakraborty <[email protected]> Co-authored-by: Jeff Yang <[email protected]> Co-authored-by: fco-dv <[email protected]>

added TimeLimit handler with its test and doc

2f5d2d7

ahmedo42 closed this Feb 4, 2021

vfdev-5 reopened this Feb 4, 2021

vfdev-5 marked this pull request as draft February 4, 2021 19:17

ahmedo42 added 3 commits February 4, 2021 22:55

fixed documentation

1831709

fixed docstring and formatting

03ab1e0

flake8 fix trailing whitespace :)

308b3a2

ahmedo42 marked this pull request as ready for review February 5, 2021 11:07

vfdev-5 reviewed Feb 5, 2021

View reviewed changes

modified class logger , default value and tests

0cd44f2

vfdev-5 requested review from fco-dv and sdesrozis February 5, 2021 14:52

changed rounding to nearest integer

58d4441

ahmedo42 requested a review from vfdev-5 February 5, 2021 15:53

vfdev-5 reviewed Feb 5, 2021

View reviewed changes

tests refactored , docs modified

902065e

vfdev-5 reviewed Feb 5, 2021

View reviewed changes

ignite/handlers/time_limit.py Outdated Show resolved Hide resolved

ahmedo42 requested a review from vfdev-5 February 5, 2021 16:59

fixed default value , removed global logger

159622b

ahmedo42 force-pushed the TimeLimit-handler branch from ceb6339 to 159622b Compare February 5, 2021 17:16

ahmedo42 and others added 2 commits February 5, 2021 19:27

fixing formatting

e589e85

Merge branch 'master' into TimeLimit-handler

f84f837

Added versionadded

000f7c3

vfdev-5 approved these changes Feb 5, 2021

View reviewed changes

sdesrozis reviewed Feb 5, 2021

View reviewed changes

ignite/handlers/time_limit.py Show resolved Hide resolved

sdesrozis reviewed Feb 5, 2021

View reviewed changes

tests/ignite/handlers/test_time_limit.py Show resolved Hide resolved

sdesrozis reviewed Feb 5, 2021

View reviewed changes

tests/ignite/handlers/test_time_limit.py Show resolved Hide resolved

ahmedo42 added 2 commits February 6, 2021 02:32

added test for engine termination

add50c7

Merge branch 'TimeLimit-handler' of https://github.com/ahmedo42/ignite …

aec7ea2

…into TimeLimit-handler

vfdev-5 requested a review from sdesrozis February 6, 2021 01:21

sdesrozis approved these changes Feb 6, 2021

View reviewed changes

sdesrozis merged commit 270bc17 into pytorch:master Feb 6, 2021

ahmedo42 deleted the TimeLimit-handler branch February 6, 2021 18:24


		from ignite.engine import Engine

		log = logging.getLogger(__name__)

Uh oh!

Conversation

ahmedo42 commented Feb 4, 2021

Uh oh!

ahmedo42 commented Feb 4, 2021

Uh oh!

vfdev-5 commented Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vfdev-5 left a comment

Choose a reason for hiding this comment

Uh oh!

vfdev-5 Feb 5, 2021

Choose a reason for hiding this comment

Uh oh!

vfdev-5 Feb 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vfdev-5 left a comment

Choose a reason for hiding this comment

Uh oh!

vfdev-5 Feb 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vfdev-5 commented Feb 5, 2021

Uh oh!

ahmedo42 commented Feb 5, 2021

Uh oh!

vfdev-5 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sdesrozis commented Feb 5, 2021

Uh oh!

sdesrozis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vfdev-5 commented Feb 4, 2021 •

edited

Loading

vfdev-5 left a comment •

edited

Loading