Tiny enhance dumper with ctx and enable flags #12622
Conversation
Summary of ChangesHello @fzyzcjy, 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 significantly upgrades the internal debugging dumper utility by introducing more sophisticated control and contextual information. It enables developers to attach global context to dumped files, dynamically manage when and what data is dumped, and provides a new utility for comprehensive tensor inspection. These enhancements aim to streamline the debugging process by offering richer metadata and flexible options for examining intermediate states within the system. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces enhancements to the _Dumper class by adding context management and an override enable flag, which improves flexibility in debugging. New methods set_ctx and override_enable are added, and the dump method is updated to incorporate these features. Additionally, a get_tensor_info utility function is introduced for more detailed tensor logging. The changes are well-structured and improve the debugging capabilities of the dumper.
| torch.set_printoptions(precision=10) | ||
| x_sample = str(x.flatten()[:5]) |
There was a problem hiding this comment.
The torch.set_printoptions calls modify global print settings, which can lead to unexpected behavior if other parts of the application rely on different print options. It's generally safer to manage print options locally or restore them after use. A try...finally block could ensure the original print options are restored.
| torch.set_printoptions(precision=10) | |
| x_sample = str(x.flatten()[:5]) | |
| original_precision = torch.get_printoptions()['precision'] | |
| try: | |
| torch.set_printoptions(precision=10) | |
| x_sample = str(x.flatten()[:5]) | |
| finally: | |
| torch.set_printoptions(precision=original_precision) |
| self._global_ctx = {} | ||
| self._override_enable = None |
There was a problem hiding this comment.
These new attributes _global_ctx and _override_enable are introduced without type hints. Adding type hints would improve code readability and maintainability, making it clearer what type of data these attributes are expected to hold.
| self._global_ctx = {} | |
| self._override_enable = None | |
| self._global_ctx: dict = {} | |
| self._override_enable: Optional[bool] = None |
| if self._forward_pass_id < 1: | ||
| print("Dump without on_forward_pass_start()") |
There was a problem hiding this comment.
The print statement for "Dump without on_forward_pass_start()" is useful for debugging, but it might be better to use a logging mechanism (e.g., logging.warning) instead of print for consistency and better control over log levels in a production environment.
| if self._forward_pass_id < 1: | |
| print("Dump without on_forward_pass_start()") | |
| if self._forward_pass_id < 1: | |
| # Consider using logging.warning here for better log management. | |
| print("Dump without on_forward_pass_start()") |
2 participants
Motivation
Modifications
Accuracy Tests
Benchmarking and Profiling
Checklist