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

make logging.logger's behavior consistent with MMLogger #1092

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

irexyc
Copy link
Collaborator

@irexyc irexyc commented Feb 1, 2024

Motivation

This PR #1064 replace MMLogger with logging.Logger, but there are two differences.

  1. MMLogger has FilterDuplicateWarning

  2. MMLogger doesn't propagate
    https://github.com/open-mmlab/mmengine/blob/main/mmengine/logging/logger.py#L292-L306
    https://github.com/python/cpython/blob/main/Lib/logging/__init__.py#L1707-L1735

lmdeploy/utils.py Outdated Show resolved Hide resolved
lmdeploy/utils.py Outdated Show resolved Hide resolved
@lvhan028 lvhan028 self-requested a review February 1, 2024 11:24
Copy link
Collaborator

@RunningLeon RunningLeon left a comment

Choose a reason for hiding this comment

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

LGTM

lvhan028
lvhan028 previously approved these changes Feb 2, 2024
@lvhan028
Copy link
Collaborator

lvhan028 commented Feb 2, 2024

The previous log

02/02 05:02:01 - turbomind - WARNING - kwargs top_p is deprecated for inference, use GenerationConfig instead.
02/02 05:02:01 - turbomind - WARNING - kwargs top_k is deprecated for inference, use GenerationConfig instead.
02/02 05:02:01 - turbomind - WARNING - kwargs temperature is deprecated for inference, use GenerationConfig instead.
02/02 05:02:01 - turbomind - WARNING - kwargs repetition_penalty is deprecated for inference, use GenerationConfig instead.
02/02 05:02:01 - turbomind - WARNING - kwargs ignore_eos is deprecated for inference, use GenerationConfig instead.
02/02 05:02:01 - turbomind - WARNING - kwargs random_seed is deprecated for inference, use GenerationConfig instead.
02/02 05:02:01 - turbomind - WARNING - kwargs request_output_len is deprecated for inference, use GenerationConfig instead.

The current log

2024-02-02 04:43:00,157 - lmdeploy.turbomind.turbomind - WARNING - kwargs top_p is deprecated for inference, use GenerationConfig instead.
2024-02-02 04:43:00,158 - lmdeploy.turbomind.turbomind - WARNING - kwargs top_k is deprecated for inference, use GenerationConfig instead.
2024-02-02 04:43:00,158 - lmdeploy.turbomind.turbomind - WARNING - kwargs temperature is deprecated for inference, use GenerationConfig instead.
2024-02-02 04:43:00,158 - lmdeploy.turbomind.turbomind - WARNING - kwargs repetition_penalty is deprecated for inference, use GenerationConfig instead.
2024-02-02 04:43:00,158 - lmdeploy.turbomind.turbomind - WARNING - kwargs ignore_eos is deprecated for inference, use GenerationConfig instead.
2024-02-02 04:43:00,158 - lmdeploy.turbomind.turbomind - WARNING - kwargs random_seed is deprecated for inference, use GenerationConfig instead.
2024-02-02 04:43:00,158 - lmdeploy.turbomind.turbomind - WARNING - kwargs request_output_len is deprecated for inference, use GenerationConfig instead

@RunningLeon RunningLeon self-requested a review February 2, 2024 05:10
@irexyc
Copy link
Collaborator Author

irexyc commented Feb 2, 2024

logger 的名字目前不是很统一,有的是指定名字,logger = get_logger('lmdeploy'),有的是根据file, logger = logging.getLogger(__name__)

@lvhan028
Copy link
Collaborator

lvhan028 commented Feb 2, 2024

不要用 __name__ 吧。

@@ -33,7 +33,7 @@

from .configuration_baichuan import BaiChuanConfig

logger = get_logger(__name__)
logger = get_logger('lmdeploy')
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we set the default value of name to 'lmdeploy' instead of None for convenience in here?

name: Optional[str] = None,

@lvhan028 lvhan028 merged commit 46ba925 into InternLM:main Feb 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants