fix: add None guard for regex in load() and update addmm_() for PyTorch >= 2.0 compatibility#551
fix: add None guard for regex in load() and update addmm_() for PyTorch >= 2.0 compatibility#551SahilKumar75 wants to merge 1 commit into
Conversation
|
Welcome @SahilKumar75! It looks like this is your first PR to kubeedge/ianvs 🎉 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: SahilKumar75 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request improves robustness in basemodel.py by escaping the dot in the regular expression and adding a safety check with a descriptive ValueError when parsing the model architecture from model_url. Additionally, it updates the addmm_ call to use keyword arguments for beta and alpha. The reviewer recommended converting model_url to a string before performing the regex search to prevent a potential TypeError if a pathlib.Path object is passed, and suggested using re.search directly instead of compiling.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| match = re.compile(r"_([a-zA-Z]+)\.pth").search(model_url) | ||
| if match is None: | ||
| raise ValueError( | ||
| f"Cannot infer model architecture from filename: '{model_url}'. " | ||
| "Expected format: '<name>_<arch>.pth'" | ||
| ) | ||
| arch = match.group(1) |
There was a problem hiding this comment.
If model_url is passed as a pathlib.Path object (which is highly possible given that Path(model_url) is used on line 70), re.search will raise a TypeError: expected string or bytes-like object. Converting model_url to a string first ensures robustness. Additionally, using re.search directly is more idiomatic than compiling a regex that is only used once.
| match = re.compile(r"_([a-zA-Z]+)\.pth").search(model_url) | |
| if match is None: | |
| raise ValueError( | |
| f"Cannot infer model architecture from filename: '{model_url}'. " | |
| "Expected format: '<name>_<arch>.pth'" | |
| ) | |
| arch = match.group(1) | |
| model_url_str = str(model_url) | |
| match = re.search(r"_([a-zA-Z]+)\.pth", model_url_str) | |
| if match is None: | |
| raise ValueError( | |
| f"Cannot infer model architecture from filename: '{model_url_str}'. " | |
| "Expected format: '<name>_<arch>.pth'" | |
| ) | |
| arch = match.group(1) |
f312f23 to
dd844e1
Compare
|
/assign @jaypume |
…ch >= 2.0 compatibility - Add None check on regex match result in load() to prevent AttributeError when model filename does not match expected '_<arch>.pth' pattern - Replace deprecated 3-positional-float addmm_() signature with keyword args (beta=1, alpha=-2) compatible with PyTorch >= 2.0 Signed-off-by: Sahil Kumar Singh <sahilkumargreat12@gmail.com>
dd844e1 to
53ba8c2
Compare
Problem
Two bugs in
examples/MOT17/multiedge_inference_bench/pedestrian_tracking/testalgorithms/reid/m3l/basemodel.pythat cause runtime failures:load()—re.compile(...).search(model_url).group(1)raisesAttributeError: 'NoneType' object has no attribute 'group'when the filename doesn't match the expected_<arch>.pthpattern._pairwise_distance()—dist_m.addmm_(1, -2, x, y.t())uses the deprecated positional-float signature removed in PyTorch ≥ 2.0, raisingTypeError.Fix
load(): capture there.search()result and raise a descriptiveValueErrorif it'sNone, before calling.group(1)._pairwise_distance(): replaceaddmm_(1, -2, x, y.t())withaddmm_(x, y.t(), beta=1, alpha=-2), which is the correct keyword-argument form supported from PyTorch 1.5+ through 2.x.Testing
Changes are logic-level; the regex fix is trivially verifiable and the
addmm_signature change matches the PyTorch ≥ 2.0 migration guide.Fixes #445