ENH: Store PEFT version in PEFT config file #2782
Conversation
This PR adds the PEFT version to the adapter_config.json. This can be useful in the future -- for instance when we change the state dict format of a PEFT method, we can convert it in a backwards compatible way based on the PEFT version being used. It can also be useful for debugging by providing an easy way to see the PEFT version that was used to train a PEFT adapter. Notes: In huggingface#2038, we made a change to PEFT configs to make it so that even if new arguments are added to a config, it can still be loaded with older PEFT versions (forward compatibility). Before that change, adding the PEFT version would have been quite disruptive, as it would make all PEFT configs incompatible with older PEFT versions. Said PR was included in the 0.14.0 release from Dec 2024, so we can expect the vast majority of PEFT users to use this version or a more recent one. If the PEFT version is a dev version, the version tag is ambiguous. Therefore, I added some code to try to determine the commit hash. This works if users installed PEFT with git+...@<HASH>. Unit testing that the function to determine the hash works with these types of installs is not trivial. Therefore, I just patched the function to return a fixed hash. I did, however, test it locally and it works: python -m pip install git+https://github.com/huggingface/diffusers.git@5e181eddfe7e44c1444a2511b0d8e21d177850a0 python -c "from peft.config import _get_commit_hash; print(_get_commit_hash('diffusers'))" Also note that I tried to make the retrieval of the hash super robust by adding a broad try ... except. If there is an error there, e.g. due to a busted install path, we never want this to fail, but rather just accept that the hash cannot be determined. If users installed a dev version of PEFT in different way, e.g. using git clone && pip install ., the commit hash will not be detected. I think this is fine, I really don't want to start shelling out with git just for this purpose.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances PEFT adapter configurations by automatically storing the PEFT version in the adapter_config.json file. This enables version tracking for debugging and future backward compatibility when PEFT state dict formats change.
Key changes:
- Automatically captures and stores PEFT version in adapter configs
- For dev versions, attempts to include commit hash for more precise version identification
- Implements robust error handling when commit hash cannot be determined
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/peft/config.py | Adds version tracking functionality with helper functions and integrates it into PeftConfigMixin |
| tests/test_config.py | Comprehensive test coverage for version storage, dev version handling, and error scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
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. |
thx copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| except Exception: | ||
| # Broad exception: We never want to break user code just because the git_hash could not be determined | ||
| warnings.warn( | ||
| "A dev version of PEFT is used but there was an error while trying to determine the commit hash. " | ||
| "Please open an issue: https://github.com/huggingface/peft/issues" | ||
| ) |
There was a problem hiding this comment.
Using a bare except Exception: is overly broad and can mask unexpected errors. Consider catching more specific exceptions like ImportError, FileNotFoundError, or JSONDecodeError that are likely to occur in the _get_commit_hash function.
tests/test_config.py
Outdated
| monkeypatch.setattr(config, "__version__", version) | ||
|
|
||
| def fake_commit_hash_raises(pkg_name): | ||
| 1 / 0 |
There was a problem hiding this comment.
Using 1 / 0 to raise an exception is unclear. Consider using raise Exception("test error") or a more descriptive exception to make the test intent clearer.
| 1 / 0 | |
| raise Exception("test error") |
src/peft/config.py
Outdated
| def _is_dev_version(version: str) -> bool: | ||
| # check if the given version is a dev version | ||
| _, _, patch = version.rpartition(".") | ||
| return patch.startswith("dev") |
There was a problem hiding this comment.
I'd vote for using packaging.version.Version(version).dev is not None here since version parsing is full of eels (or something like that). Example: 1.2.3dev0 is a valid python version but would fail to be detected here as dev version.
src/peft/config.py
Outdated
| "A dev version of PEFT is used but there was an error while trying to determine the commit hash. " | ||
| "Please open an issue: https://github.com/huggingface/peft/issues" | ||
| ) | ||
| git_hash = None |
There was a problem hiding this comment.
does it make sense to specifically mark this, e.g. {version}@UNKNOWN? i can imagine this behavior being unhelpful when something goes wrong in a dev install but we report a non-dev version number.
- use packaging.Version to determine dev - add @unknown if hash cannot be determined - clearer error in test
This PR adds the PEFT version to the
adapter_config.json. This can be useful in the future -- for instance when we change the state dict format of a PEFT method, we can convert it in a backwards compatible way based on the PEFT version being used. It can also be useful for debugging by providing an easy way to see the PEFT version that was used to train a PEFT adapter.Notes
In #2038, we made a change to PEFT configs to make it so that even if new arguments are added to a config, it can still be loaded with older PEFT versions (forward compatibility). Before that change, adding the PEFT version would have been quite disruptive, as it would make all PEFT configs incompatible with older PEFT versions. Said PR was included in the 0.14.0 release from Dec 2024, so we can expect the vast majority of PEFT users to use this version or a more recent one.
If the PEFT version is a dev version, the version tag is ambiguous. Therefore, I added some code to try to determine the commit hash. This works if users installed PEFT with
git+...@<HASH>. Unit testing that the function to determine the hash works with these types of installs is not trivial. Therefore, I just patched the function to return a fixed hash. I did, however, test it locally and it works:python -m pip install git+https://github.com/huggingface/diffusers.git@5e181eddfe7e44c1444a2511b0d8e21d177850a0 python -c "from peft.config import _get_commit_hash; print(_get_commit_hash('diffusers'))"Also note that I tried to make the retrieval of the hash super robust by adding a broad
try ... except Exception. If there is an error there, e.g. due to a busted install path, we never want this to fail, but rather just accept that the hash cannot be determined.If users installed a dev version of PEFT in different way, e.g. using
git clone && pip install ., the commit hash will not be detected. I think this is fine, I really don't want to start shelling out with git just for this purpose.