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

Save detection inference logs to file #12042

Merged
merged 4 commits into from
May 29, 2024

Conversation

AlexPasqua
Copy link
Contributor

I added the possibility to save the detection logs to file.

The code was almost all there already, just some arguments weren't passed the right way in order to save the detection logs to file.

In case, we could also implement this feature for the other scripts in tools/infer/.

Copy link

paddle-bot bot commented May 2, 2024

Thanks for your contribution!

@CLAassistant
Copy link

CLAassistant commented May 2, 2024

CLA assistant check
All committers have signed the CLA.

log_file = args.save_log_path
if os.path.isdir(args.save_log_path) or (not os.path.exists(args.save_log_path) and args.save_log_path.endswith("/")):
log_file = os.path.join(log_file, "benchmark_detection.log")
logger = get_logger(log_file=log_file)
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems this logger should be prepared and passed into AutoLog as a arg in https://github.com/PaddlePaddle/PaddleOCR/blob/main/tools/infer/predict_det.py#L168

I am worrying about maintenance of AutoLog module, it's been implemented in https://github.com/LDOUBLEV/AutoLog, and published to a server as documented here:

${python_name} -m pip install https://paddleocr.bj.bcebos.com/libs/auto_log-1.2.0-py3-none-any.whl
.

I think this is only been used by a terminated tipc plan, we should redesign this feature, or deprecate this feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am worrying about maintenance of AutoLog module, it's been implemented in https://github.com/LDOUBLEV/AutoLog, and published to a server as documented here:

In this case PaddleOCR could implement its own logger, instead of using AutoLog.

I think that it's a little bit of a pity to have a feature to run benchmarks without the option to save the results to a file (and only seeing them on screen instead), therefore I think this feature would be very useful.

Copy link
Collaborator

Choose a reason for hiding this comment

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

agree

Copy link
Collaborator

@jzhang533 jzhang533 left a comment

Choose a reason for hiding this comment

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

LGTM

@jzhang533 jzhang533 merged commit 6e7a1b8 into PaddlePaddle:main May 29, 2024
3 checks passed
luzhongqiu pushed a commit to luzhongqiu/PaddleOCR that referenced this pull request Jun 3, 2024
* Save detection inference logs to file

* Minor fix on save_log_path

* Formatted with black

* If logger is None: get_logger()
@luotao1
Copy link
Collaborator

luotao1 commented Oct 15, 2024

@AlexPasqua Thanks for your contribution! You will receive a beautiful PaddlePaddle gift. Please provide your mailing address by filling out the following questionnaire before October 18th.

Looking forward to the future, we will walk further together in the world of open source!
Click Here :https://paddle.wjx.cn/vm/h4On9gJ.aspx#

@luotao1
Copy link
Collaborator

luotao1 commented Oct 18, 2024

@AlexPasqua Could you provide your phone number? We need this information when mailing gifts. Please send your phone number to ext_paddle_oss@baidu.com. Thanks very much!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants