[Oneshot refactor] Refactor initialize_model_from_path#1109
Merged
Conversation
|
👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review. Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed. |
This was referenced Feb 3, 2025
dsikka
reviewed
Feb 4, 2025
kylesayrs
reviewed
Feb 4, 2025
kylesayrs
requested changes
Feb 4, 2025
Collaborator
kylesayrs
left a comment
kylesayrs
left a comment
There was a problem hiding this comment.
Please move set_seed outside of the initialize_model_from_path function. Otherwise lgtm.
Author
Its a part of training args, https://github.com/huggingface/transformers/blob/main/src/transformers/training_args.py#L1044 and in the original code. If there is an argument to move it outside, we can do it in a follow up pr. |
dsikka
added a commit
that referenced
this pull request
Feb 5, 2025
…ame_from_model (#1108) ORDER OF REVIEWS: 1. #1108 <- current PR 2. #1103 3. #1109 4. #1110 SUMMARY: * Rename `get_shared_processor_src` to `get_processor_from_model` * Appropriate signature on `initialize_processor_from_path`, where `teacher` should be optinal TEST PLAN: * Pass all existing tests * Search `get_shared_processor_src` using pygrep ```bash 3 function pygrep() { 2 local search_term="$1" 1 shift 126 local search_dirs="${*:-src examples tests}" 1 grep -rn --include="*.py" -E "$search_term" $search_dirs 2 } ``` --------- Co-authored-by: Dipika Sikka <dipikasikka1@gmail.com>
dsikka
pushed a commit
that referenced
this pull request
Feb 11, 2025
ORDER OF REVIEWS: 1. #1108 2. #1103 <- current PR 3. #1109 4. #1110 SUMMARY: Refactor dataclass used for llm-compressor entrypoints (oneshot, train, apply) to decouple non-relevant attributes from the existing dataclass. Ex. recipe in training_args. Recipe is contained in a session, not in the trainer that training_args govern. Dataclass refactor details are in https://docs.google.com/document/d/1YbR1dTQmCzqhGk74m5msBzqoPHQgB6dVxDtf6cTmetc/edit?usp=sharing Note: #1110 takes care of using a new entrypoint that will prohibit the post_train / oneshot call to use training_args. Current entrypoint will need training_args for oneshot to function - this PR is just for refactoring the dataclass. Before: ModelArguments: https://github.com/vllm-project/llm-compressor/blob/6fa5a5eecc7d363ec73474d011d40135b6374179/src/llmcompressor/transformers/finetune/model_args.py#L6 DataTrainingArguments: https://github.com/vllm-project/llm-compressor/blob/6fa5a5eecc7d363ec73474d011d40135b6374179/src/llmcompressor/transformers/finetune/data/data_args.py#L70 TrainingArguments: https://github.com/vllm-project/llm-compressor/blob/6fa5a5eecc7d363ec73474d011d40135b6374179/src/llmcompressor/transformers/finetune/training_args.py#L10 After: ModelArguments: https://github.com/vllm-project/llm-compressor/pull/1103/files#diff-58fd0f7ae4564317960ae0d4d4b2cdb97b9588c1915f062915e74ecf51b5502cR6 DatasetArguments: https://github.com/vllm-project/llm-compressor/pull/1103/files#diff-5e43f74ba5d8327b937adada3c7f30a7efb13f9a44cb3fdb5e1a2a12b8b8ea27R70 RecipeArguments: https://github.com/vllm-project/llm-compressor/pull/1103/files#diff-0ff9c048a4deb55e5459054bdc61a5d8c81da9c94588ec2355e6b2c2ec8675d1R6 TrainingArguments: https://github.com/vllm-project/llm-compressor/pull/1103/files#diff-249ee96763dd50956a7309f898eda4bcaa91c6af653474568fbda10b5a39c817R12 TEST PLAN: * Pass all existing tests * Search dataclass arguments using `pygrep` ```bash 3 function pygrep() { 2 local search_term="$1" 1 shift 126 local search_dirs="${*:-src examples tests}" 1 grep -rn --include="*.py" -E "$search_term" $search_dirs 2 } ``` --------- Co-authored-by: Rahul Tuli <rahul@neuralmagic.com>
brian-dellabetta
approved these changes
Feb 11, 2025
Collaborator
brian-dellabetta
left a comment
brian-dellabetta
left a comment
There was a problem hiding this comment.
things look good to me, just breaking change concerns but looks like you addressed those in the slack thread with @robertgshaw2-redhat
kylesayrs
approved these changes
Feb 11, 2025
dsikka
pushed a commit
that referenced
this pull request
Feb 24, 2025
ORDER OF REVIEWS: 1. #1108 2. #1103 3. #1109 4. #1110 <- current PR SUMMARY: * Create a class to decouple dependency to `main`. Class `Oneshot` consists of pre-processing, carrying out oneshot logic and post-processing * Move the oneshot class and method under `llmcompressor/entrypoints/oneshot.py`. * Add ReadMe in `/llmcompressor/entrypoints` for info on oneshot * Delete oneshot logic from `/finetune` directory, add deprecation warning * Remove apply used only for stagerunner oneshot pathway in session.py and session_function.py * Add oneshot only calibration dataloader logic * Add a return variable of `model: PretrainedModel` for `def oneshot`. * Make oneshot carryout logic independent of `TrainingArguments` * remove `overwrite_output_dir` as oneshot input arg -> only used for `TrainingArguments` * Update README on `/finetune` path. Remove `oneshot` logic and `oneshot with fsdp` * Update `wrap_save_pretrained` logic to run only if not updated already -> used for stage runner to avoid double wrapping Entrypoints: ```python3 from llmcompressor import oneshot oneshot(**kwargs) # calls Oneshot ``` or ```python3 from llmcompressor import Oneshot oneshot = Oneshot(**kwargs) oneshot() # preprocesss, carries out logic and post process ``` TEST PLAN: Pass all tests and examples. Verified https://github.com/vllm-project/llm-compressor/blob/main/examples/quantization_2of4_sparse_w4a16/llama7b_sparse_w4a16.py works as expected. FOLLOW UPS: * Stage runner removal * Update entrypoints folder with train, eval, predict, etc. --------- Signed-off-by: George Ohashi <george@neuralmagic.com> Signed-off-by: George <george@neuralmagic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
ORDER OF REVIEWS:
SUMMARY:
Refactor
initialize_model_from_pathto decoupletraining_argsdependent logic and oneshot (non-training_args) logic.TEST PLAN:
initialize_model_from_pathusinggrep