[Feature] Add SegmindLoggerHook #1650

saurbhc · 2022-01-09T13:58:09Z

Motivation

I would like to use https://github.com/segmind/segmind as a Logger.

Modification

I added a new LoggerHook child to the HOOKS registry.

Use cases (Optional)

Logging training parameters with Segmind.

CLAassistant · 2022-01-09T13:58:14Z

All committers have signed the CLA.

zhouzaida · 2022-01-10T02:19:32Z

Hi @saurbhc , thanks for your contribution. We can refer to https://github.com/open-mmlab/mmcv/blob/master/tests/test_runner/test_hooks.py#L1212 to add a unit test.

- add SegmindLoggerHook import in: mmcv/runner/__init__.py mmcv/runner/hooks/__init__.py mmcv/runner/hooks/logger/__init__.py

saurbhc · 2022-01-10T11:32:28Z

@zhouzaida I have added a unit test.

mmcv/runner/hooks/logger/segmind.py

- Add Docstring to SegmindLoggerHook - Use get_loggable_tags(...)

mmcv/runner/hooks/logger/segmind.py

tests/test_runner/test_hooks.py

- mmcv/runner/hooks/logger/segmind.py moved docs from __init__ to class ... update ImportError line-indentation remove unwanted method - tests/test_runner/test_hooks.py update assert_called_with only on hook.segmind_mlflow_log

saurbhc · 2022-01-11T15:45:54Z

@zhouzaida
lint check is failing because of:

running pre-commit for the last commit on the file tests/test_runner/test_hooks.py is showing some "cyclic-
dependency" by 'isort' and 'yapf'.

Here's the output of pre-commit:

$ pre-commit
flake8...................................................................Passed
seed isort known_third_party.............................................Passed
isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing tests/test_runner/test_hooks.py

yapf.....................................................................Failed
- hook id: yapf
- files were modified by this hook
Trim Trailing Whitespace.................................................Passed
Check Yaml...........................................(no files to check)Skipped
Fix End of Files.........................................................Passed
Fix requirements.txt.................................(no files to check)Skipped
Fix double quoted strings................................................Passed
Check for merge conflicts................................................Passed
Fix python encoding pragma...............................................Passed
Mixed line ending........................................................Passed
Markdownlint.........................................(no files to check)Skipped
codespell................................................................Passed
docformatter.............................................................Passed

To reproduce this, run pre-commit on this PR for the last commit.

zhouzaida · 2022-01-12T08:13:32Z

@zhouzaida lint check is failing because of:

running pre-commit for the last commit on the file tests/test_runner/test_hooks.py is showing some "cyclic-
dependency" by 'isort' and 'yapf'.

Here's the output of pre-commit:

$ pre-commit
flake8...................................................................Passed
seed isort known_third_party.............................................Passed
isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing tests/test_runner/test_hooks.py

yapf.....................................................................Failed
- hook id: yapf
- files were modified by this hook
Trim Trailing Whitespace.................................................Passed
Check Yaml...........................................(no files to check)Skipped
Fix End of Files.........................................................Passed
Fix requirements.txt.................................(no files to check)Skipped
Fix double quoted strings................................................Passed
Check for merge conflicts................................................Passed
Fix python encoding pragma...............................................Passed
Mixed line ending........................................................Passed
Markdownlint.........................................(no files to check)Skipped
codespell................................................................Passed
docformatter.............................................................Passed

To reproduce this, run pre-commit on this PR for the last commit.

We can disable the yapf.

tests/test_runner/test_hooks.py

mmcv/runner/hooks/logger/segmind.py

disable yapf on test_hooks.py imports Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

Update SegmindLoggerHook docstring Co-authored-by: Jiazhen Wang <47851024+teamwong111@users.noreply.github.com>

removed un-used statements

mmcv/runner/hooks/logger/segmind.py

- code cleanup

teamwong111

Could you please successfully run a task with segmind and show it in the pr content? There are few segmind documents, and AWS is required for configuration, verification is troublesome for me.

mmcv/runner/hooks/logger/segmind.py

- add interval parameter in __init__ method

saurbhc · 2022-01-19T18:13:37Z

@teamwong111 I have verified SegmindLoggerHook on Segmind Platform and it's working fine.

I am attaching these for your reference:

Verifying with mmcv
.ipynb link .ipynb file used in Segmind (added some debugging prints for verifying the logging)
Segmind Metrics screenshot link
Verifying with mmdetection
.ipynb link .ipynb file used in Segmind
Segmind Metrics screenshot link

zhouzaida · 2022-01-20T06:23:36Z

@teamwong111 I have verified SegmindLoggerHook on Segmind Platform and it's working fine.

I am attaching these for your reference:

Verifying with mmcv
.ipynb link .ipynb file used in Segmind (added some debugging prints for verifying the logging)
Segmind Metrics screenshot link

Verifying with mmdetection
.ipynb link .ipynb file used in Segmind
Segmind Metrics screenshot link

More arguments should also be passed.

mmcv/mmcv/runner/hooks/logger/base.py

Lines 14 to 20 in b6e1ab7

    
               Args: 
        
                   interval (int): Logging interval (every k iterations). Default 10. 
        
                   ignore_last (bool): Ignore the log of last iterations in each epoch 
        
                       if less than `interval`. Default True. 
        
                   reset_flag (bool): Whether to clear the output buffer after logging. 
        
                       Default False. 
        
                   by_epoch (bool): Whether EpochBasedRunner is used. Default True.

- more arguments passes to __init__ method - interval - ignore_last - reset_flag - by_epoch

saurbhc · 2022-01-28T09:21:12Z

Can someone please review this?

zhouzaida

LGTM

saurbhc · 2022-02-02T23:37:42Z

Hey @zhouzaida, Can we merge this PR?

zhouzaida · 2022-02-03T01:36:12Z

Hey @zhouzaida, Can we merge this PR?

Definitely.
Please @ZwwWayne have a look.

HAOCHENYE

LGTM.

mmcv/runner/hooks/logger/segmind.py

Co-authored-by: Mashiro <57566630+HAOCHENYE@users.noreply.github.com>

ZwwWayne · 2022-03-01T07:45:59Z

Hi @saurbhc ,
It seems that the unit test fails. Could you fix them so that we can merge this PR?

add SegmindLoggerHook

c50cb5e

saurbhc added 2 commits January 9, 2022 16:55

update linting for segmind.py

1c468ff

ran pre-commit

71b348b

add test_segmind_hook

8cb559b

- add SegmindLoggerHook import in: mmcv/runner/__init__.py mmcv/runner/hooks/__init__.py mmcv/runner/hooks/logger/__init__.py

zhouzaida reviewed Jan 10, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

zhouzaida reviewed Jan 10, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Show resolved Hide resolved

update test_segmind_hook

dcf7810

- Add Docstring to SegmindLoggerHook - Use get_loggable_tags(...)

zhouzaida requested a review from teamwong111 January 11, 2022 14:21

zhouzaida reviewed Jan 11, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

zhouzaida reviewed Jan 11, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

zhouzaida reviewed Jan 11, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

zhouzaida reviewed Jan 11, 2022

View reviewed changes

tests/test_runner/test_hooks.py Outdated Show resolved Hide resolved

update test_hooks.py & segmind.py

00ab387

- mmcv/runner/hooks/logger/segmind.py moved docs from __init__ to class ... update ImportError line-indentation remove unwanted method - tests/test_runner/test_hooks.py update assert_called_with only on hook.segmind_mlflow_log

zhouzaida reviewed Jan 12, 2022

View reviewed changes

tests/test_runner/test_hooks.py Outdated Show resolved Hide resolved

teamwong111 reviewed Jan 12, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

saurbhc and others added 3 commits January 13, 2022 16:11

Update tests/test_runner/test_hooks.py

cb897e4

disable yapf on test_hooks.py imports Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

Update mmcv/runner/hooks/logger/segmind.py

21d8d64

Update SegmindLoggerHook docstring Co-authored-by: Jiazhen Wang <47851024+teamwong111@users.noreply.github.com>

update before_run method in segmind.py

6621fab

removed un-used statements

saurbhc force-pushed the feat/segmind_hook branch from 18cd85b to 6621fab Compare January 13, 2022 16:26

updated imports in SegmindLoggerHook

0a04ae3

saurbhc requested a review from zhouzaida January 14, 2022 18:44

zhouzaida reviewed Jan 15, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

update SegmindLoggerHook

6b1b679

- code cleanup

saurbhc requested a review from zhouzaida January 16, 2022 14:45

zhouzaida requested a review from ZwwWayne January 17, 2022 08:05

teamwong111 reviewed Jan 18, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

saurbhc added 2 commits January 19, 2022 16:38

Merge branch 'open-mmlab:master' into feat/segmind_hook

a0fc570

update SegmindLoggerHook

0ea8ebe

- add interval parameter in __init__ method

update SegmindLoggerHook

2aea4a1

- more arguments passes to __init__ method - interval - ignore_last - reset_flag - by_epoch

zhouzaida requested a review from HAOCHENYE January 23, 2022 15:04

zhouzaida approved these changes Jan 28, 2022

View reviewed changes

HAOCHENYE approved these changes Feb 11, 2022

View reviewed changes

teamwong111 approved these changes Feb 18, 2022

View reviewed changes

zhouzaida approved these changes Feb 18, 2022

View reviewed changes

HAOCHENYE reviewed Feb 23, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

HAOCHENYE reviewed Feb 23, 2022

View reviewed changes

mmcv/runner/hooks/logger/segmind.py Outdated Show resolved Hide resolved

saurbhc and others added 2 commits February 23, 2022 12:30

Update mmcv/runner/hooks/logger/segmind.py

2ef2690

Co-authored-by: Mashiro <57566630+HAOCHENYE@users.noreply.github.com>

Update mmcv/runner/hooks/logger/segmind.py

86b4454

Co-authored-by: Mashiro <57566630+HAOCHENYE@users.noreply.github.com>

zhouzaida approved these changes Mar 1, 2022

View reviewed changes

Merge branch 'open-mmlab:master' into feat/segmind_hook

3801f1b

saurbhc force-pushed the feat/segmind_hook branch from f63cffa to 3801f1b Compare March 1, 2022 11:55

zhouzaida approved these changes Mar 3, 2022

View reviewed changes

zhouzaida changed the title ~~add SegmindLoggerHook~~ [Feature] Add SegmindLoggerHook Mar 3, 2022

zhouzaida merged commit 34f227e into open-mmlab:master Mar 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Add SegmindLoggerHook #1650

[Feature] Add SegmindLoggerHook #1650

saurbhc commented Jan 9, 2022

CLAassistant commented Jan 9, 2022 •

edited

Loading

zhouzaida commented Jan 10, 2022

saurbhc commented Jan 10, 2022

saurbhc commented Jan 11, 2022

zhouzaida commented Jan 12, 2022

teamwong111 left a comment

saurbhc commented Jan 19, 2022 •

edited

Loading

zhouzaida commented Jan 20, 2022

saurbhc commented Jan 28, 2022

zhouzaida left a comment

saurbhc commented Feb 2, 2022

zhouzaida commented Feb 3, 2022 •

edited

Loading

HAOCHENYE left a comment

ZwwWayne commented Mar 1, 2022

[Feature] Add SegmindLoggerHook #1650

[Feature] Add SegmindLoggerHook #1650

Conversation

saurbhc commented Jan 9, 2022

Motivation

Modification

Use cases (Optional)

CLAassistant commented Jan 9, 2022 • edited Loading

zhouzaida commented Jan 10, 2022

saurbhc commented Jan 10, 2022

saurbhc commented Jan 11, 2022

zhouzaida commented Jan 12, 2022

teamwong111 left a comment

Choose a reason for hiding this comment

saurbhc commented Jan 19, 2022 • edited Loading

zhouzaida commented Jan 20, 2022

saurbhc commented Jan 28, 2022

zhouzaida left a comment

Choose a reason for hiding this comment

saurbhc commented Feb 2, 2022

zhouzaida commented Feb 3, 2022 • edited Loading

HAOCHENYE left a comment

Choose a reason for hiding this comment

ZwwWayne commented Mar 1, 2022

CLAassistant commented Jan 9, 2022 •

edited

Loading

saurbhc commented Jan 19, 2022 •

edited

Loading

zhouzaida commented Feb 3, 2022 •

edited

Loading