[misc] feat: optimize console output#29
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on optimizing console output and improving the user experience by reducing noise and unnecessary warnings. It achieves this by refining data filtering logic, decreasing the verbosity of progress indicators, and enhancing the package's main entry point for cleaner execution. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request aims to optimize console output by lazy-loading the main entry point, filtering trace events, and reducing progress log frequency. The changes are generally positive and align with the stated goals. However, I've identified a critical issue in the logic for reducing log frequency which can lead to a ZeroDivisionError when processing a small number of ranks. I have provided a code suggestion to fix this bug. The other changes appear to be correct and beneficial.
| if completed % (total_ranks // 10) == 0: | ||
| logger.info( | ||
| f"Completed rank {rank_id}: {completed}/{total_ranks} ({progress:.1f}%)" | ||
| ) |
There was a problem hiding this comment.
The expression total_ranks // 10 can result in zero if total_ranks is less than 10. This will cause a ZeroDivisionError when calculating completed % 0. To prevent this crash, you should ensure the divisor is at least 1. A concise way to fix this is to use (total_ranks // 10 or 1), which defaults to 1 if the integer division result is 0.
| if completed % (total_ranks // 10) == 0: | |
| logger.info( | |
| f"Completed rank {rank_id}: {completed}/{total_ranks} ({progress:.1f}%)" | |
| ) | |
| if completed % (total_ranks // 10 or 1) == 0: | |
| logger.info( | |
| f"Completed rank {rank_id}: {completed}/{total_ranks} ({progress:.1f}%)" | |
| ) |
What does this PR do?
Checklist Before Starting
[{modules}] {type}: {description}(This will be checked by the CI){modules}includemstx,mvtx,torch_profile,deployment,perf,algo,env,doc,data,cfg,ci,misc,,like[mstx, ci]{type}is infeat,fix,refactor,chore,test[BREAKING]to the beginning of the title.[BREAKING][mstx, torch_profile] feat: support timeline parsingTest
API and Usage Example
# Add code snippet or script demonstrating how to use thisDesign & Code Changes
Checklist Before Submitting
Important
Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always