Repository navigation
fix: Add prefer_auto_map opt-in for registered-class vs auto_map conflicts - #45729
sebastianvlad1 wants to merge 3 commits into
Conversation
|
Hi, AutoConfig.from_pretrained can now select dynamic loading for a local config file input, but it still passes the file path into get_class_from_dynamic_module, which only treats directories as local and will mis-handle the file path as a Hub repo id. Severity: action required | Category: correctness How to fix: Normalize file path to dir Agent prompt to fix - you can give this to your LLM of choice:
Found by Qodo code review. FYI, Qodo is free for open-source. |
73824b3 to
412b2e3
Compare
|
@Rocketknight1 would you mind taking a look when you get a chance? |
412b2e3 to
d983c73
Compare
|
[For maintainers] Suggested jobs to run (before merge) run-slow: auto |
|
cc @hmellor since I haven't investigated this too deeply and you were commenting on the original issue, but if you want me to take it instead, just ping me! |
|
Sorry for taking so long to review! I read through the code but tbh, it would be easier for code review if code formatting was applied in a separate commit and the real beef of the change kept separate from that. Similarly, I would've dropped the commit that doesn't relate to prefer_auto_map fix, since that shouldn't be needed anymore, right? Maybe HF has some policy on when is the right time to apply code formatting changes, maybe it's ok to keep them in the same commit.. but it increases the code review load a fair bit. With these changes, how does the original repro from #45698 change behavior? |
|
Thanks for the review @nurpax! Yes, the intent here was to implement the direction suggested in #45698: expose the precedence explicitly with a per call prefer_auto_map option, while preserving the current default behavior from #45094. So the behavior is:
|
What does this PR do?
This PR keeps the default precedence introduced in #45094: explicitly registered local classes continue to override checkpoint
auto_mapcode by default.It adds an explicit
prefer_auto_map=Trueopt-in toAutoConfig.from_pretrained()for cases where callers want the checkpoint'sauto_mapcode to win instead.AutoModel.from_pretrained(..., prefer_auto_map=True)supports the same workflow when the config is auto-loaded, because the flag is forwarded intoAutoConfig.from_pretrained().The PR also preserves the local config-file-path fix from the earlier revision by normalizing file paths to their containing directory before calling
get_class_from_dynamic_module().Fixes #45698
Code Agent Policy
Before submitting
Pull Request section?
to it if that's the case.
from_pretrained loads wrong custom module after save_pretrained #45698
documentation guidelines, and
here are tips on formatting docstrings.
Validation
Who can review?
@Cyrilvallez @Rocketknight1 @nurpax