DOC Fix broken docstring examples in peft_model and tuner models - #3657
Open
blipbyte wants to merge 1 commit into
Open
DOC Fix broken docstring examples in peft_model and tuner models#3657blipbyte wants to merge 1 commit into
blipbyte wants to merge 1 commit into
Conversation
Four of the six `PeftModelFor*` docstring examples in `peft_model.py` raise `TypeError` when run as written, because they pass config fields removed in 2023: - `postprocess_past_key_value_function`, removed in a7dd034 (2023-02-08), in the SequenceClassification, CausalLM and TokenClassification examples. - `enable_lora`, removed in c21afbe (2023-03-28), in the Seq2SeqLM example. Both fields were deleted rather than renamed, so the lines are removed with no replacement key. All six classes are pulled into the API reference via `[[autodoc]]`, so these are the examples a reader copies off the docs site. `PeftModelForTokenClassification` had a second problem behind the first: it imported `AutoModelForSequenceClassification` but called `AutoModelForTokenClassification`. The import now matches the class. The lora, hira and beft examples constructed the tuner class directly (`LoraModel(model, config, "default")`), which returns an object with no `save_pretrained` — a reader who copies it cannot save their adapter. Switched to `get_peft_model`, matching the form used in ia3 and adalora since huggingface#3254. Also dropped the unused `PeftModel` import from the lora and hira k-bit examples, and `peft_type="Beft"` from beft, which `BeftConfig.__post_init__` silently overwrites and whose value did not match the enum (`PeftType.BEFT` is "BEFT"). No runtime behavior change; all edits inside docstrings.
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
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.
Fixes #3644. Approved by @BenjaminBossan in
#3644 (comment).
Docstrings only. No runtime code is touched.
peft_model.py— four examples that cannot runFour of the six
PeftModelFor*examples raiseTypeErrorwhen run aswritten, because they pass config fields that were removed in 2023:
postprocess_past_key_value_function, removed ina7dd0347(2023-02-08),appears in the
PeftModelForSequenceClassification,PeftModelForCausalLMand
PeftModelForTokenClassificationexamples.enable_lora, removed inc21afbe8(2023-03-28), appears in thePeftModelForSeq2SeqLMexample.Both fields were deleted rather than renamed, so there is no replacement key
and the lines are simply removed. The remaining keys still describe the
configuration each example demonstrates.
All six classes are pulled into the API reference via
[[autodoc]], so theseare the examples a reader copies off the docs site.
PeftModelForTokenClassificationhad a second problem behind the first: itimported
AutoModelForSequenceClassificationbut calledAutoModelForTokenClassification, so it would still have failed with aNameErroronce the config was valid. The import now matches the class.lora,hira,beft— examples that misleadThese three construct the tuner class directly:
That silently writes a base-model checkpoint instead of an adapter:
BaseTuner.__getattr__forwards to the wrapped module, sosave_pretrainedresolves toPreTrainedModel.save_pretrainedrather than failing. Switched toget_peft_model, matching the form you asked for in the #3254 review foria3andadalora; these three were never swept.Also removed the unused
PeftModelimport from theloraandhirak-bitexamples.
beftadditionally passedpeft_type="Beft", whichBeftConfig.__post_init__overwrites with
PeftType.BEFT. It was the only example where the value wasalso wrong — the enum is
"BEFT", not"Beft", and the mismatch is silent.The same redundant
peft_type=appears inprompt_tuning,p_tuning,ia3,adaloraandprefix_tuning, but there the string matches the enum,so I left them out of scope. Happy to remove those too if you'd prefer.
Testing
Nothing in CI executes these examples —
make testrunspytest tests/, andno doctest collection touches
src/. That is why they sat broken since 2023.So I ran them directly.
Before: 4 of 6 fail with the
TypeErrors above. After: all six build theirconfig, and five run end to end. The sixth,
PeftModelForCausalLM, needsgpt2-large, which is not in my local cache.The three tuner examples were run as edited too. Each now returns a
PeftModel, sosave_pretrainedwrites an adapter. The direct form resolvessave_pretrainedtoPreTrainedModel.save_pretrainedand writes a base-model checkpoint instead — no error either way.Also run:
make quality(ruff, format, doc-builder, doc coverage 131/131) andpytest tests/test_config.py tests/test_decoder_models.py— 7064 passed,2718 skipped.
Per the issue discussion, no test that executes the examples was added —
@BenjaminBossan said it wasn't needed.
AI assistance: I used Claude Code to help audit and draft this. I reviewed
every changed line, ran every command above myself, and can defend the change
end-to-end.