Add positional args to PeftModelForCausalLM.generate #1393
Conversation
| _maybe_include_all_linear_layers, | ||
| check_target_module_exists, | ||
| inspect_matched_modules, | ||
| ) | ||
| from peft.utils import INCLUDE_LINEAR_LAYERS_SHORTHAND |
There was a problem hiding this comment.
This is a minor edit to import the constant from the right place
| @@ -672,10 +686,6 @@ def _test_generate_half_prec(self, model_id, config_cls, config_kwargs): | |||
| # check if `generate` works | |||
| _ = model.generate(input_ids=input_ids, attention_mask=attention_mask) | |||
|
|
|||
| with self.assertRaises(TypeError): | |||
| # check if `generate` raises an error if no positional arguments are passed | |||
| _ = model.generate(input_ids, attention_mask=attention_mask) | |||
There was a problem hiding this comment.
This is a redundant check on positional args (behaviour is captured in test_generate already) so I've removed it
There was a problem hiding this comment.
Thank you @SumanthRH for adding support for positional args to generate method of CausalLM. LGTM! 🤗
Given that the messy internals of input processing are handled in the prepare_inputs_for_generation method, I think this is a safe feature.
Yes, while reviewing the PR I was thinking the same, makes sense.
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
There was a problem hiding this comment.
This is very nice and fixes un-consistency between PreTrainedModel API and PeftModel classes ! Thanks a lot @SumanthRH !
* add positional args * update tests
What does this PR do?
A small PR for adding positional arguments to the generate method for
PeftModelForCausalLM. The motivation is thatPeftModelclasses likePeftModelForCausalLMshould feel more like a wrapper forAutoModeland shouldn't break existing code which uses positional args. Given that the messy internals of input processing are handled in theprepare_inputs_for_generationmethod, I think this is a safe feature. Also, it looks likePeftModelForSeq2SeqLMneeds some refactoring for the prompt learning methods (to shift logic from.generatetoprepare_inputs_for_generation), so I'm leaving that refactoring + positional args support to another PR.