Feature/raw text dataprep #3612
Conversation
Summary of ChangesHello @Vangmay, 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 expands the data input capabilities of the training pipeline by enabling direct training on raw text files. It introduces a robust 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
This pull request introduces a valuable feature for training on raw text files by adding a RawTextDataLoader. The implementation is well-structured, supporting various file formats and integrating with the CLI. However, I've identified a few critical issues that prevent the feature from working as intended. The most significant problem is that the new data loading logic in unsloth-cli.py is defined but never actually called, and the new RawTextDataLoader class is not properly exported, which will lead to an import error. Additionally, there's a new CLI argument that is unused and a notable performance inefficiency in the text chunking process. My review includes detailed feedback and suggestions to address these points.
| # First pass: tokenize the entire text to get accurate token counts | ||
| tokenized = self.tokenizer(text, return_tensors="pt", add_special_tokens=False) | ||
| tokens = tokenized["input_ids"] |
There was a problem hiding this comment.
The current implementation reads and tokenizes the entire file at once. This approach can lead to very high memory consumption for large files (e.g., several gigabytes), potentially causing out-of-memory errors. For better scalability, consider implementing a streaming approach where the file is read and processed in smaller chunks instead of loading everything into memory.
for more information, see https://pre-commit.ci
…y/unsloth into feature/raw-text-dataprep
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| from transformers import TrainingArguments | ||
| from unsloth import is_bfloat16_supported | ||
| import logging | ||
| from unsloth import RawTextDataLoader |
There was a problem hiding this comment.
The CLI now imports RawTextDataLoader directly from unsloth, but unsloth/__init__.py never re-exports that class (it only exists in unsloth/dataprep/raw_text.py), so running the script will raise ImportError: cannot import name 'RawTextDataLoader' before any training begins and the new --raw_text_file flow is unusable until the package exposes the symbol or the import is made module-qualified.
Useful? React with 👍 / 👎.
unsloth/dataprep/__init__.py
Outdated
| # limitations under the License. | ||
|
|
||
| from .synthetic import * | ||
| from raw_text import * |
There was a problem hiding this comment.
In the new dataprep package init, from raw_text import * attempts to import a top-level module rather than the sibling unsloth.dataprep.raw_text; in a normal install this raises ModuleNotFoundError when importing unsloth.dataprep, so RawTextDataLoader/TextPreprocessor cannot be reached or re-exported.
Useful? React with 👍 / 👎.
| # Move to next chunk with stride overlap | ||
| if end_idx == len(tokens): | ||
| break | ||
| start_idx += chunk_size - stride |
There was a problem hiding this comment.
The chunking loop advances start_idx by chunk_size - stride without guarding against a stride equal to or larger than the chunk size; if a caller passes such values (allowed by the CLI flags), start_idx never increases and while start_idx < len(tokens) will loop indefinitely on multi-chunk inputs, hanging tokenization.
Useful? React with 👍 / 👎.
|
Hello @danielhanchen, apologies for the delay. I have made the edits mentioned by codex! |
2 participants
Fixes #14
Enables training directly on raw text files without requiring structured datasets. Adds
RawTextDataLoaderclass with intelligent token-aware chunking, support for multiple formats (.txt, .md, .json, .jsonl, .csv), and CLI integration with--raw_text_fileflag.Usage:
python unsloth-cli.py --raw_text_file book.txt --chunk_size 1024Test:
python tests/test_raw_text.py