Multi Adapter Fixes #472
Closed
Multi Adapter Fixes #472
Conversation
This file contains 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
…into add-multiple-adapters
Co-authored-by: lewtun <[email protected]>
remove extra files
use the peft config to do this more cleanly remove torch.no_grad as we now don't have any params that require grad in compute_reward_score
fixed model.score incorrect dtype - changed to pretrained_model.dtype added policy_adapter_name explicitly to __init__ refactored compute_reward_score - moved into ppo_trainer - changed to use the .forward method of your model so that accelerator.unwrap is not necessary any more - changed score / seq_cls head use same code as llamaforseqcls - made outputs a scalar per input example instead of a matrix where you need to select [-1, 0] removed unnecessary list comprehensions for `any`
mnoukhov
changed the title
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
There was a problem hiding this comment.
Hi @mnoukhov
Thanks a lot for the PR ! currently a CI test fails:
FAILED tests/test_ppo_trainer.py::PPOTrainerTester::test_peft_model_ppo_adapter_rm_trainer - TypeError: add_and_load_reward_modeling_adapter() missing 1 required positional argument: 'adapter_model_id'Can you please double check? 🙏 Thanks!
|
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread. |
|
Sorry I forgot about this PR. Re-merged with main and re-opening. Should have fixed the test error |
|
@younesbelkada can you re-open this or should I make a new PR? |
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.
Multi-Adapter RL, as it currently is, does not reproduce StackLLaMA results and has some design choices that can be improved. This PR fixes some basics and improves the design. This is the first step in the StackLLaMA repro #471 but I won't add more to the current PR to keep it manageable.
the main changes are
compute_reward_scoreto use model.forward so it works seamlessly with accelerate in multi-gpu, this can be changed to just accelerator.unwrap_model otherwisemodel.scorelayer's dtypemodel.pretrained_modelrequires_grad = Falseby usinginference_mode = Truein the rm_adapter_peft_config