Skip to content

fix: explain why a legacy YOLO checkpoint fails to unpickle - #523

Open
roshaninfordham wants to merge 1 commit into
roboflow:mainfrom
roshaninfordham:fix/actionable-error-for-legacy-yolo-checkpoints
Open

fix: explain why a legacy YOLO checkpoint fails to unpickle#523
roshaninfordham wants to merge 1 commit into
roboflow:mainfrom
roshaninfordham:fix/actionable-error-for-legacy-yolo-checkpoints

Conversation

@roshaninfordham

Copy link
Copy Markdown

Description

Partially addresses #357.

That issue reports two failures in sequence. The first — weights_only defaulting to True in PyTorch 2.6 — is already fixed: every torch.load call in model_processor.py now passes weights_only=False. The second is still live. After downgrading torch, the reporter hit:

ModuleNotFoundError: No module named 'models'

This is not a roboflow bug in the usual sense, and that is exactly the problem: nothing in the message says so. yolov5, yolov7 and yolov9 checkpoints pickle their model classes by reference (models.yolo, utils.*) rather than by value, so unpickling only succeeds in an environment where the training repository is importable. Run deploy() from any other directory and pickle fails on a module the user has no reason to connect to their own checkpoint.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)

Motivation and Context

I could not make the checkpoint load without the training repo — that is inherent to how pickle-by-reference works, and adding a sys.path shim would mean guessing where the user's yolov5 clone lives. What is fixable is that the failure gives no indication of the cause or the remedy.

So this fails loudly instead of cryptically:

Could not load weights/best.pt: the checkpoint references the module 'models', which is
not importable here. Checkpoints produced by the yolov5, yolov7 and yolov9 training
repositories store their model classes by reference, so they can only be unpickled from
an environment where that repository is importable. Run the upload from the training
repository's directory, or add it to PYTHONPATH.

If you would prefer roboflow to attempt the sys.path injection itself, that is a bigger change and a different discussion — I would rather ask than guess. This one is strictly an error-message improvement and changes no successful path.

Changes Made

  • roboflow/util/model_processor.py_load_checkpoint catches ModuleNotFoundError and re-raises it as ModelPackagingError, naming error.name and the remedy, chained with from error so the original traceback survives. All three call sites (740, 1065, 1182) go through this helper, so they are all covered.
  • tests/util/test_model_processor.py — a TestLoadCheckpointErrors case class: the translation, a pass-through of a successful load including the exact torch.load kwargs, and a check that unrelated exceptions are not swallowed.

Testing

  • I have tested this code locally
  • I have added unit tests that prove my fix is effective
  • All new and existing tests pass

I first confirmed the mechanism against real torch rather than assuming it. Building a checkpoint that pickles a class from a module named models, then loading it from a directory where models is not importable:

checkpoint written, pickles models.yolo.DetectionModel
REPRODUCED: ModuleNotFoundError: No module named 'models'

That is the reporter's error exactly. The committed tests then drive _load_checkpoint with an injected torch double, so they need no torch dependency and no checkpoint fixture — the helper already takes the torch module as a parameter.

Against unmodified main the translation test errors with the untranslated exception:

ModuleNotFoundError: No module named 'models'
FAILED (errors=1)
python -m unittest              before: 970 tests, OK (skipped=1)
python -m unittest              after:  973 tests, OK (skipped=1)
ruff check roboflow/ tests/     All checks passed!
ruff format --check             2 files already formatted

Google Colab (optional)

Not applicable; this is an error-path change covered by unit tests, with the underlying mechanism verified locally as shown above.

yolov5/v7/v9 checkpoints pickle their model classes by reference, so
torch.load only resolves them when the training repository is importable.
Outside it, deploy() failed with a bare

    ModuleNotFoundError: No module named 'models'

which names neither the checkpoint nor anything the user can act on.

Translate it into ModelPackagingError, naming the missing module and what
to do about it, and chain the original with `from error`. Other load
failures are left to propagate unchanged.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

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.

1 participant