Fix/transformers eval #1191
Merged
Fix/transformers eval #1191
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a TypeError exception handler in the backup function to address issues occurring when integrating with transformers, specifically when using Ctrl+C and tqdm does not exit immediately.
Refactored the logic for removing control sequences from log lines by introducing the remove_control_sequences function, which handles both carriage returns and ANSI escape sequences more robustly. Updated clean_control_chars to use this new function and expanded unit tests to cover various edge cases for control sequence removal.
Introduces argparse to allow customization of sample count and sequence length from the command line. Updates output directory handling, adds evaluation strategy and steps, and improves overall script flexibility for Qwen2 model fake training.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Updated the ANSI escape sequence regex for more accurate matching and modified the clean_control_chars function to remove empty lines from the result. This improves the cleanliness and readability of processed log output.
Replaces standalone test functions for clean_control_chars with a class-based approach using static methods. Adds more granular and descriptive test cases to improve coverage and clarity.
Updated the utils.print_watch call to use self.run_store.swanlog_dir, ensuring the correct directory is referenced when printing watch information after run completion.
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes multiple issues related to evaluation and logging in the SwanLab framework, particularly focusing on transformers integration and multi-threading stability.
- Fixes threading issues with tqdm causing handle conflicts in multi-threaded environments
- Resolves garbled log output issues in transformers framework evaluation
- Improves test scripts and fixes incorrect watch directory paths in local environment
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/log/test_log.py | Rewrites and expands unit tests for control character cleaning functions |
| test/integration/transformers/transformers_fake_train.py | Enhances transformers integration test with configuration options and evaluation strategy |
| swanlab/log/log.py | Improves control character cleaning with new regex pattern and dedicated function |
| swanlab/data/porter/init.py | Adds TypeError exception handling for tqdm threading issues |
| swanlab/data/callbacker/local.py | Fixes incorrect watch directory path from run_dir to swanlog_dir |
Comments suppressed due to low confidence (5)
test/unit/log/test_log.py:269
- The test method name is missing the 'test_' prefix which is required for pytest to recognize it as a test method.
def removes_escape_sequence_and_returns_content_after():
test/unit/log/test_log.py:274
- The test method name is missing the 'test_' prefix which is required for pytest to recognize it as a test method.
def returns_original_line_if_no_control_sequence():
test/unit/log/test_log.py:279
- The test method name is missing the 'test_' prefix which is required for pytest to recognize it as a test method.
def handles_multiple_control_sequences_and_returns_content_after_last():
test/unit/log/test_log.py:284
- The test method name is missing the 'test_' prefix which is required for pytest to recognize it as a test method.
def handles_empty_string_and_returns_empty():
test/unit/log/test_log.py:289
- The test method name is missing the 'test_' prefix which is required for pytest to recognize it as a test method.
def handles_only_control_sequence_and_returns_empty():
Zeyi-Lin
approved these changes
Jul 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #1118