Skip to content

fix: guard model.to() calls against backend proxies in inference and validation#305

Open
imagra93 wants to merge 2 commits into
LibreYOLO:devfrom
imagra93:fix/backend-proxy-to-guard
Open

fix: guard model.to() calls against backend proxies in inference and validation#305
imagra93 wants to merge 2 commits into
LibreYOLO:devfrom
imagra93:fix/backend-proxy-to-guard

Conversation

@imagra93
Copy link
Copy Markdown
Contributor

InferenceRunner._set_device() in models/base/inference.py and GazeInferenceRunner._set_device() in models/l2cs/inference.py both call self.model.model.to(device) unconditionally. When the inner model is a _BackendEvalProxy (ONNX / TensorRT backend), the proxy has no .to() method — device management is the runtime's responsibility — so the call raises AttributeError and prediction fails.

Fix: wrap both .to() calls with a hasattr guard. model.device is still updated so the wrapper reflects the intended device; only the PyTorch-specific .to() transfer is skipped for proxies.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 621cf18c0b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread libreyolo/validation/base.py
@EHxuban11
Copy link
Copy Markdown
Contributor

This changes behaviour of the shared validator and thus affects every single model, which worries me. I think that such big change could come after the v1.2.0 release.

This is the pushback that codex wrote while I was reviewing the PR:

This changes behavior in the shared `BaseValidator._setup()` path, so it affects every
validator/model, not only backend proxy inference.

Specifically, validation now moves `self.model.model` to `self.device` and overwrites
`self.model.device`. That may be the right fix for native PyTorch validation device
overrides, but it is a broader behavior change than the proxy `.to()` crash this PR
describes.

I reproduced a regression with `TorchScriptBackend`: after validator setup,
`backend.device` changes from the string `"cpu"` to `torch.device("cpu")`. That then
changes later backend device comparisons/warnings, because backend code treats
`device` as a runtime string.

For v1.2.0 I’d prefer the narrow fix: guard the two inference `.to()` calls, and
either drop the shared validator mutation or split it into a follow-up PR with
backend-preservation tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants