Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTDETR implementation #940

Merged
merged 17 commits into from
Apr 25, 2024
Merged

RTDETR implementation #940

merged 17 commits into from
Apr 25, 2024

Conversation

edugzlez
Copy link
Contributor

I have included a basic wrapper to use RTDETR (which is implemented in the ultralytics library).

RTDETR: https://docs.ultralytics.com/models/rtdetr/

I had to change the behavior of the device parameter because it was not responding correctly. In this case, I pass it directly to the ultralytics prediction method.

I have included the demo and the download functions of the base model.

@fcakyon
Copy link
Collaborator

fcakyon commented Nov 5, 2023

Amazing contribution @edugzlez ! If you could also include some tests for RTDETR (you can check the yolov8 tests), we would gladly merge your PR 👍

Copy link
Collaborator

@fcakyon fcakyon left a comment

Choose a reason for hiding this comment

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

unittests needed

@Nisse123
Copy link

Very interested in RT-DETR support, please complete this PR

@devrimcavusoglu
Copy link
Member

devrimcavusoglu commented Jan 9, 2024

@edugzlez Can you add the commits on this PR edugzlez#1 and we can merge this PR.

Moreover, about removing device placement on Yolov8DetectionModel, it's kind of undesired. My proposal is to overwrite it on RTDETRDetectionModel

@edugzlez
Copy link
Contributor Author

edugzlez commented Jan 17, 2024

Sorry @devrimcavusoglu for the delay. I've already accepted the PR and refreshed the branch.

Thank you

@devrimcavusoglu
Copy link
Member

Thank you @edugzlez, I mentioned 1 more issue in the comment that changing the device behavior in Yolov8DetectionModel class could be undesired. Can you move that device change to RTDETRDetectionModel and update the PR ? Also, add a comment to that device change line why it is needed. Then, we can merge the PR 👍

@felipetobars
Copy link

@edugzlez Hi, how could I start testing ultralytics' SAHI implementation for RTDETR? since it is not yet incorporated into the library when installing it with pip

@edugzlez edugzlez requested a review from fcakyon April 25, 2024 14:01
@devrimcavusoglu
Copy link
Member

@edugzlez Thank you for the amazing contribution. 🎉

@devrimcavusoglu devrimcavusoglu merged commit f75f04a into obss:main Apr 25, 2024
9 checks passed
@felipetobars
Copy link

Thank you! @edugzlez

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.

5 participants