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

Add ultralytics SAM support when using bboxes and points arguments in SAM().predict() #1454

Merged
merged 6 commits into from
Aug 15, 2024

Conversation

xaristeidou
Copy link
Contributor

@xaristeidou xaristeidou commented Aug 15, 2024

Description

Fixes issue noted in #1453 .

Added brief check and fill of necessary fields in sv.Detections()

List any dependencies that are required for this change.

No new dependencies

Please delete options that are not relevant.

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

How has this change been tested, please provide a testcase or example of how you tested the change?

Colab attached for testing

Docs

  • Docs updated? Not needed

Colab notebook for testing: https://colab.research.google.com/drive/10aRaBAfb__xrMeBNn9amYwrKgcnbExBn?usp=sharing

@xaristeidou
Copy link
Contributor Author

xaristeidou commented Aug 15, 2024

class_id=np.arange(len(ultralytics_results)), : This is due to the fact that in ultralytics Results if there are more than one masks, their class_id is labeled with increasing +1 step start from 0 (e.g. 0,1,2, etc).

@onuralpszr onuralpszr added the bug Something isn't working label Aug 15, 2024
Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>
@onuralpszr onuralpszr force-pushed the add-ultralytics-SAM-support branch 2 times, most recently from 7277ab4 to ab45b19 Compare August 15, 2024 19:19
@xaristeidou
Copy link
Contributor Author

Why does pre-commit check fails, mask_to_xyxy is imported properly and runs as expected locally. Locally in my system pre-commit pass all checks.

@onuralpszr
Copy link
Collaborator

onuralpszr commented Aug 15, 2024

@xaristeidou I also added mask to xyxy and pre-commit locally fine at my end. Probably some server issue. I tested with normal cases such as yolov8 model not with sam and works well. LGTM to me !

@onuralpszr
Copy link
Collaborator

Why does pre-commit check fails, mask_to_xyxy is imported properly and runs as expected locally. Locally in my system pre-commit pass all checks.

I think server issue I have no idea

@onuralpszr
Copy link
Collaborator

@xaristeidou I am going to update your branch to latest dev and test that out before merge maybe pre-commit will act fine I will backup and revert in case something happen.

Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>
Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>
@onuralpszr
Copy link
Collaborator

@xaristeidou I understand why act weird because in our CURRENT develop we removed that import so compare to latest makes fail so I had to update repo and re-add again to fix it. Now it is all good.

@xaristeidou
Copy link
Contributor Author

@onuralpszr Nice! Makes sense now.

@onuralpszr
Copy link
Collaborator

I am merging in. Thank you so much @xaristeidou !

@onuralpszr onuralpszr merged commit eecc0de into roboflow:develop Aug 15, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants