Skip to content

Patch MPS torch.gather to avoid errors on changing max_det#17057

Closed
Y-T-G wants to merge 9 commits intoultralytics:mainfrom
Y-T-G:torch_gather_mps
Closed

Patch MPS torch.gather to avoid errors on changing max_det#17057
Y-T-G wants to merge 9 commits intoultralytics:mainfrom
Y-T-G:torch_gather_mps

Conversation

@Y-T-G
Copy link
Collaborator

@Y-T-G Y-T-G commented Oct 20, 2024 •
edited by UltralyticsAssistant
Loading

Resolves #17054

Patch based on dbolya/tomesd#15

MRE

from ultralytics import YOLO, ASSETS

model = YOLO("yolov10n.pt")
results = model(ASSETS / "bus.jpg", max_det=1, device="mps")

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Added a fix for a torch.gather bug on macOS with Metal Performance Shaders (MPS), improving compatibility. 🛠️🍎

📊 Key Changes

  • Introduced a custom torch_gather patch to address a known MPS-related issue in torch.gather.
  • Applied the patch conditionally for macOS systems (MACOS check).

🎯 Purpose & Impact

  • Purpose: Fix a bug with torch.gather when running PyTorch on macOS using MPS, ensuring stability and correctness.
  • Impact: macOS users (especially those leveraging Apple Silicon) benefit from bug-free tensor operations with MPS, enabling smoother training and inference workflows. 🚀

@UltralyticsAssistant UltralyticsAssistant added bug Something isn't working as intended in the official Ultralytics package. enhancement New feature or request fixed Bug has been resolved labels Oct 20, 2024
@UltralyticsAssistant
Copy link
Member

UltralyticsAssistant commented Oct 20, 2024

👋 Hello @Y-T-G, thank you for submitting your pull request to the ultralytics/ultralytics 🚀 repository! This is an automated response to help streamline your contribution process, and a member of the Ultralytics team will assist you shortly. In the meantime, please review the following checklist to ensure everything is set for a smooth integration of your patch:

  • ✅ Purpose Definition: Make sure the purpose of your patch, which addresses the MPS torch.gather bug, is clearly articulated in the PR description. Also, link it to the reported issue.
  • ✅ Sync with Main Branch: Please ensure your branch is updated with the latest changes from the main branch. You can do this by clicking 'Update branch' or running git pull and git merge main locally.
  • ✅ CI Checks: Verify that all Continuous Integration (CI) checks have passed. If there are any failures, please resolve them as soon as possible. Check out our CI documentation for guidance.
  • ✅ Documentation Update: Ensure that any changes are documented so users can understand how these adjustments may affect their usage. You can update relevant documentation as necessary.
  • ✅ Testing: Add or update tests that confirm your patch works as intended, and ensure all current tests pass.
  • ✅ Sign the CLA: If this is your first contribution to Ultralytics, please sign our Contributor License Agreement by writing "I have read the CLA Document and I sign the CLA" in a comment.
  • ✅ Scope of Changes: Keep your changes minimal and focused on fixing the torch.gather issue only. As Bruce Lee wisely put it, "It is not daily increase but daily decrease, hack away the unessential."

If you need further guidance, don't hesitate to consult our Contributing Guide. Thank you for helping make Ultralytics even better! 🚀✨

@codecov
Copy link

codecov bot commented Oct 20, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.04%. Comparing base (69c12a4) to head (598dd0d).

Files with missing lines Patch % Lines
ultralytics/utils/patches.py 75.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #17057      +/-   ##
==========================================
+ Coverage   74.01%   74.04%   +0.03%     
==========================================
  Files         127      127              
  Lines       17099    17105       +6     
==========================================
+ Hits        12656    12666      +10     
+ Misses       4443     4439       -4
Flag Coverage Δ
Benchmarks 35.05% <85.71%> (+0.05%) ⬆️
GPU 39.12% <42.85%> (-0.01%) ⬇️
Tests 67.79% <85.71%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Y-T-G Y-T-G changed the title Patch MPS torch.gather to avoid errors Patch MPS torch.gather to avoid errors on changing max_det Oct 21, 2024
Burhan-Q
Burhan-Q approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@Burhan-Q Burhan-Q left a comment

Choose a reason for hiding this comment

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

LGTM

@Y-T-G Y-T-G closed this Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@glenn-jocher glenn-jocher Awaiting requested review from glenn-jocher

1 more reviewer

@Burhan-Q Burhan-Q Burhan-Q approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working as intended in the official Ultralytics package. enhancement New feature or request fixed Bug has been resolved

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

YOLOv10s trained on RHEL 8 with CUDA support crashes upon inference with device = 'mps'

3 participants

@Y-T-G @UltralyticsAssistant @Burhan-Q