fix(cli): recognize pretrained model aliases in format_model_suffix#5719
Conversation
CLI deep-eval commands (dp test/eval-desc/embed/model-devi) normalize their -m
argument through format_model_suffix(), which decided support by testing
Path(filename).suffix against the registered backend suffixes. PretrainedBackend
registers built-in model names as whole-name (lowercased) suffix aliases such as
"dpa-3.2-5m", so Path("DPA-3.2-5M").suffix == ".2-5M" never matched and
DPA3-Omol-Large has an empty suffix. With a preferred_backend always supplied by
the CLI, the unmatched alias silently got the default backend suffix appended
(e.g. "DPA-3.2-5M" -> "DPA-3.2-5M.pth") and the subsequent load failed, even
though the pretrained backend advertises those names. The Python API
(DeepPot("DPA-3.2-5M")) worked because Backend.detect_backend_by_model matches
with a case-insensitive endswith, so the two matchers were inconsistent.
Align format_model_suffix with Backend.match_filename: match on a
case-insensitive endswith over the suffix set, which recognizes both ordinary
dotted suffixes and whole-name pretrained aliases and returns them unchanged.
Adds tests asserting aliases (DPA-3.2-5M, DPA3-Omol-Large, lowercase) pass
through unchanged, with controls that an ordinary ".pth" is preserved and an
unknown bare name still gets the preferred suffix appended. Before the fix the
alias case failed (mangled to "DPA-3.2-5M.pth").
Fix deepmodeling#5673
📝 WalkthroughWalkthroughThe ChangesSuffix Matching Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Related issues: Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deepmd/backend/suffix.py (1)
65-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire exact match for bare aliases
Backend.match_filenameand this helper both useendswith, so filenames likemydpa-3.2-5mare accepted as the pretrained aliasdpa-3.2-5m. Keependswithfor dotted extensions, but use case-insensitive equality for bare aliases so unintended names don’t get dispatched as valid models.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deepmd/backend/suffix.py` around lines 65 - 76, The filename matching in the suffix helper is too permissive for bare aliases because the current endswith check allows names like mydpa-3.2-5m to match a registered alias. Update the logic in the Path/filename normalization path so dotted extensions still use case-insensitive endswith, but bare alias entries are matched with case-insensitive exact equality only. Keep the behavior aligned with Backend.match_filename and detect_backend_by_model while tightening the alias handling in the helper that returns the normalized filename or appends preferred_backend.suffixes[0].
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@deepmd/backend/suffix.py`:
- Around line 65-76: The filename matching in the suffix helper is too
permissive for bare aliases because the current endswith check allows names like
mydpa-3.2-5m to match a registered alias. Update the logic in the Path/filename
normalization path so dotted extensions still use case-insensitive endswith, but
bare alias entries are matched with case-insensitive exact equality only. Keep
the behavior aligned with Backend.match_filename and detect_backend_by_model
while tightening the alias handling in the helper that returns the normalized
filename or appends preferred_backend.suffixes[0].
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ae6d291f-d52a-40ac-aa6f-8d23652e603e
📒 Files selected for processing (2)
deepmd/backend/suffix.pysource/tests/common/test_pretrained_backend.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5719 +/- ##
==========================================
- Coverage 81.26% 81.13% -0.13%
==========================================
Files 988 988
Lines 110877 110877
Branches 4234 4237 +3
==========================================
- Hits 90103 89964 -139
- Misses 19249 19386 +137
- Partials 1525 1527 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Problem
Fixes #5673. CLI deep-eval commands (
dp test,dp eval-desc,dp embed,dp model-devi) normalize their-margument throughformat_model_suffix(), which decided whether a name is supported by comparingPath(filename).suffixagainst the registered backend suffixes.PretrainedBackendregisters built-in model names as whole-name (lowercased) suffix aliases such asdpa-3.2-5m, butPath("DPA-3.2-5M").suffixis.2-5M(andDPA3-Omol-Largehas an empty suffix), so the alias never matched. Because the CLI always passes apreferred_backend, the unmatched alias did not raise; it silently got the default backend suffix appended (DPA-3.2-5MbecomesDPA-3.2-5M.pth) and the subsequent load failed.Note this refines the issue's description: the observed failure is a silent suffix-mangle, not the
ValueError("Unsupported model file format")the issue predicts — that raise branch is unreachable from these CLI commands because a preferred backend is always supplied. The Python API (DeepPot("DPA-3.2-5M")) already worked, becauseBackend.detect_backend_by_modelmatches with a case-insensitiveendswith; the two matchers were simply inconsistent.Fix
Align
format_model_suffixwithBackend.match_filename: match on a case-insensitiveendswithover the suffix set. This recognizes both ordinary dotted suffixes (.pth,.pb, ...) and whole-name pretrained aliases (dpa-3.2-5m,dpa3-omol-large) and returns the recognized name unchanged, soPretrainedBackendcan resolve it downstream.Test
Adds tests to
source/tests/common/test_pretrained_backend.pyasserting thatDPA-3.2-5M,DPA3-Omol-Large, and a lowercase alias pass throughformat_model_suffixunchanged, with controls that an ordinarymodel.pthis preserved and an unknown bare name still receives the preferred suffix. The alias case fails on master (mangled toDPA-3.2-5M.pth) and passes with the fix. This path had no prior coverage.Summary by CodeRabbit
Bug Fixes
Tests