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.

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

Logging update #267

Merged
merged 10 commits into from
Jan 21, 2020
Merged

Logging update #267

merged 10 commits into from
Jan 21, 2020

Conversation

blisc
Copy link
Collaborator

@blisc blisc commented Jan 15, 2020

Updated as of PR #311:
Updates the our use of get_logger such that it now defaults to logging.getLogger("nemo_logger") instead of logging.getLogger('')
Updates the our use of get_logger such that it now defaults to logging.getLogger("nemo") instead of logging.getLogger('')
Adds the package level variable nemo.logging which can and should be used in place of neuralFactory.logger
Users should use this logger by adding the following line to their code: from nemo import logging and then use the logger as per usual, eg logging.info("Message")
Default verbosity is set at INFO for process 0
Logger files are still automatically created when neural factory is initialized
The logging formatted was changed to include file and line number that generated the message

Signed-off-by: Jason <jasoli@nvidia.com>
Signed-off-by: Jason <jasoli@nvidia.com>
Signed-off-by: Jason <jasoli@nvidia.com>
Signed-off-by: Jason <jasoli@nvidia.com>
Signed-off-by: Jason <jasoli@nvidia.com>
Signed-off-by: Jason <jasoli@nvidia.com>
Signed-off-by: Jason <jasoli@nvidia.com>
Signed-off-by: Jason <jasoli@nvidia.com>
@blisc blisc marked this pull request as ready for review January 16, 2020 02:15
Signed-off-by: Jason <jasoli@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Jan 16, 2020

This pull request fixes 4 alerts when merging 9f83e1a into 2bb07bc - view on LGTM.com

fixed alerts:

  • 3 for Unused import
  • 1 for Wrong name for an argument in a call

return get_logger('')
else:
return self.action.logger
warnings.warn("This will be deprecated in future releases. Please use "
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -805,7 +800,9 @@ def optim_level(self):

@property
def logger(self):
return self._exp_manager.logger
warnings.warn("This will be deprecated in future releases. Please use "
Copy link
Contributor

Choose a reason for hiding this comment

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

hmmmm, we had that definition already in callbacks.py...

anyway, we will get rid both of them soon, so not really important ;)

Copy link
Contributor

@tkornuta-nvidia tkornuta-nvidia left a comment

Choose a reason for hiding this comment

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

Hey @blisc thanks for taking care of that, great job!

My only comments are that some of those could be changed to debug/warnings. I have indicated all the places where I think it should be done.

@okuchaiev if you think those changes are out of the scope please say so...

@tkornuta-nvidia tkornuta-nvidia removed the request for review from yzhang123 January 16, 2020 22:17
Signed-off-by: Jason <jasoli@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented Jan 21, 2020

This pull request introduces 1 alert and fixes 5 when merging 0200c9e into bcc94a8 - view on LGTM.com

new alerts:

  • 1 for Unused import

fixed alerts:

  • 4 for Unused import
  • 1 for Wrong name for an argument in a call

@blisc blisc merged commit bc0d7b1 into NVIDIA:master Jan 21, 2020
@blisc blisc deleted the u_logging_update branch January 25, 2020 01:20
dcurran90 pushed a commit to dcurran90/NeMo that referenced this pull request Oct 15, 2024
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