Skip to content

[seq2seq] fix logger format for non-main process#9911

Merged
LysandreJik merged 1 commit into
huggingface:masterfrom
stas00:logger-dist
Feb 1, 2021
Merged

[seq2seq] fix logger format for non-main process#9911
LysandreJik merged 1 commit into
huggingface:masterfrom
stas00:logger-dist

Conversation

@stas00
Copy link
Copy Markdown
Contributor

@stas00 stas00 commented Jan 31, 2021

Currently, in finetune_trainer.py non-main process doesn't have any formatting at all, so we end up with:

[WARNING|modeling_t5.py:1645] 2021-01-30 20:01:37,246 >> [p0] got MPU
[WARNING|modeling_t5.py:1646] 2021-01-30 20:01:37,246 >> [p0] DP group [0]
[p1] got MPU
[p1] DP group [1]

as you can see the 2nd process in DDP misses formatting in logger.

this PR fixes it.

I looked in the take-over version run_seq2seq.py if it needed to be fixed too and it doesn't have these function calls, not sure why. They appear to be needed, unless they get called elsewhere.

@sgugger, @patil-suraj

@sgugger
Copy link
Copy Markdown
Collaborator

sgugger commented Jan 31, 2021

@LysandreJik knows better for the centralized logging system so I'll defer to him.

@stas00 stas00 requested a review from LysandreJik January 31, 2021 16:54
Copy link
Copy Markdown
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Indeed, good catch! All processes should share the handler/formatting options, even if they have different verbosity.

Thanks @stas00!

@LysandreJik LysandreJik merged commit 6bab836 into huggingface:master Feb 1, 2021
@stas00 stas00 deleted the logger-dist branch February 1, 2021 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants