fix(yolo-skill): don't require ultralytics on mps when framework_ok is False (backport to master) - #211
Closed
solderzzc wants to merge 1 commit into
Closed
fix(yolo-skill): don't require ultralytics on mps when framework_ok is False (backport to master)#211solderzzc wants to merge 1 commit into
solderzzc wants to merge 1 commit into
Conversation
…s False load_optimized()'s final fallback branch (taken when the optimized runtime check fails) unconditionally imported ultralytics. mps installs deliberately ship without torch/ultralytics (requirements_mps.txt: only onnxruntime is needed), so any mps machine where the CoreML EP check fails crashed with "No module named 'ultralytics'" instead of falling back to the pre-built ONNX model via plain CPUExecutionProvider. Now mps uses _load_onnx_coreml() (already used elsewhere in this same function) in that branch instead, matching the file's own stated intent. Addresses one of the symptoms in #207. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Member
Author
|
Closing — this repo's branch strategy only allows develop→master PRs (per CI's check-target job). Reopening the same fix as a develop→master PR instead. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backport of #210 (already merged to
develop) tomaster.masteris the branch the Aegis-AI app's skill installer actually clones from at install/reinstall time (skill-registry-service.cjsfetchesskills.jsonoffmasterandgit sparse-checkouts the skill from there) —developisn't in that path at all, so #210 alone doesn't reach any installed skill. This PR ports the identical fix tomasterso a skill reinstall actually picks it up.load_optimized()'s final fallback branch (taken when the optimized runtime check fails,framework_ok=False) unconditionally imported ultralytics — but mps installs deliberately never ship torch/ultralytics (requirements_mps.txtonly listsonnxruntime)._check_mps_runtime()fails (CoreML execution provider not found) crashed withNo module named 'ultralytics'when loading a YOLO skill, instead of falling back to the pre-built.onnxmodel via plainCPUExecutionProvider.Failed to load model: No module named 'ultralytics',backend=mps,framework_ok=False) reproduces this path exactly.Fix
On the mps backend, that fallback branch now calls
_load_onnx_coreml()(already used elsewhere in the same function for the mps happy path) against the pre-built.onnx, instead of assuming ultralytics is importable. If no pre-built.onnxexists either, it raises a clearRuntimeErrorinstead of a confusingModuleNotFoundError.Applied identically to both copies of
env_config.py(skills/detection/yolo-detection-2026/scripts/andskills/lib/), matching how #194 touched both.Test plan
skills/lib/test_env_config_mps_fallback.py(3 tests) re-verified against currentmaster— passes.skills/lib/test_env_config_*.pysuite onmaster— 4 pre-existing failures intest_env_config_ane.pyreproduce identically onmasterwithout this change (unrelated to this fix, not introduced by it; verified by temporarily reverting just the twoenv_config.pyfiles back tomaster's version and re-running).yolo-detection-2026skill from the Aegis-AI Skills sidebar to pick it up.🤖 Generated with Claude Code