Chronos-2: Ensure base model revision is saved in LoRA adapter config #432
Conversation
Description: This commit fixes a critical issue where the base model's revision was failing to be recorded in `adapter_config.json` during LoRA fine-tuning. Previously, `fit()` did not propagate the loaded model's revision to `LoraConfig`, resulting in the `revision` field being missing or None in the saved adapter configuration. This omission meant that loading the saved adapter would silently default to the `main` branch of the base model, rather than the specific revision used during training, breaking reproducibility. Key changes: * Persist Revision: Store the `revision` passed to `from_pretrained` in `model.config._source_revision`. * Propagate to Config: Update `fit()` to inject this stored revision into `LoraConfig` so it is correctly saved in `adapter_config.json`. * Validate Integrity: Raise `ValueError` if an explicit `revision` argument in `fit()` conflicts with the loaded model's source revision. Impact: EEnsures that adapter_config.json always contains the correct base model revision. This guarantees that AutoGluon can faithfully reproduce the model state by simply loading the adapter, without risking a silent version mismatch, even in scenarios where the base model receives minimal or infrequent updates.
ryuichi-ichinose
force-pushed
the
feature/save-revision
branch
from
07fd331 to
7f2f5a1
Compare
|
@ryuichi-ichinose Thanks for the PR. I understand the problem and I have some comments.
I am happy to hear your opinions on why you think this feature is important. |
|
Related: huggingface/peft#1658 |
|
@abdulfatir Reproducibility is the cornerstone of both research and production, and it is proven by data integrity—not by "plans." |
|
@abdulfatir Even if updates are effectively non-existent, my argument is not about the fear of updates, but about the explicit definition of integrity. Since this is an optional argument, it adds zero friction for casual users. Furthermore, allowing users to specify the revision effectively offloads the responsibility of version management from you to us. However, if you still don't trust this approach, then just close the PR. |
|
@ryuichi-ichinose Thanks for your POV. We need to test a few things before we merge this, so this likely won't be part of the next release. If versioning is a issue for you at the moment, I would recommend downloading the model from HF and using a local path. That way, you won't need to worry about potential problems upstream. |
|
@abdulfatir OK. I got it. |
2 participants
Description of changes:
Fix reproducibility: save base model revision in adapter_config
The Problem Currently, fit() does not propagate the base model's revision to the LoraConfig. As a result, adapter_config.json is saved without the revision. When loading this adapter later, it silently defaults to the main branch of the base model. If the base model updates, the adapter becomes incompatible or produces different results, breaking reproducibility.
Even if the base model receives minimal or infrequent updates, explicit versioning is strictly necessary.
Key changes:
-Pipeline: explicitly store revision in Chronos2Pipeline instance variables.
-Loading: pass revision to the constructor in from_pretrained.
-Training: inject self.revision into LoraConfig during fit().
Impact:
adapter_config.json will now correctly persist the base model revision. This guarantees that AutoGluon can reproduce the exact model state.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.