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

added onnxruntime and upgrade ultralytics #14

Merged
merged 2 commits into from
Mar 26, 2024
Merged

Conversation

mchtey
Copy link
Contributor

@mchtey mchtey commented Mar 14, 2024

No description provided.

@isms
Copy link
Contributor

isms commented Mar 15, 2024

@mchtey Thanks for the PR. One question - is it possible to add the package you want without changing the ultralytics package? We would like to keep the environment stable to avoid impacting others who may be using ultralytics.

I see the versions suggest this is just a patch update (8.1.11 to 8.1.27), so alternatively if you can look at the changelog for ultralytics and flag anything that may be problematic or explain why nothing should impact others we can consider it.

@mchtey
Copy link
Contributor Author

mchtey commented Mar 16, 2024

I also want to work with YoloV9. I think you may want to update your ultralytics version as Yolov9 has been officially added to version 8.1.23. I don't think this version will negatively affect other colleagues. :)

@isms
Copy link
Contributor

isms commented Mar 17, 2024

@mchtey The build has failed with the proposed change though, see here: https://github.com/drivendataorg/spacecraft-pose-object-detection-runtime/actions/runs/8310616225/job/22760366309?pr=15#step:4:803

Please make sure you're able to build this locally (make build) before requesting re-review.

@isms
Copy link
Contributor

isms commented Mar 20, 2024

@mchtey Are you still interested in getting this PR merged? Let us know if you want to keep it open.

@mchtey
Copy link
Contributor Author

mchtey commented Mar 21, 2024

image

Hi,
It seems that there is an issue related to the ncnn library here. However, there is no such change in the pull request (PR) I have submitted. Could you please double-check? I acknowledge that I'm a novice, but the issue might not be in my PR. It appears that the ncnn extension has been added in a PR submitted after mine.
Thank you.

@isms
Copy link
Contributor

isms commented Mar 21, 2024

@mchtey That version is not available yet: https://anaconda.org/conda-forge/ultralytics/labels

@mchtey
Copy link
Contributor Author

mchtey commented Mar 21, 2024

I asked the developers and this is what they told me. "Oh yea looks like updates haven't gotten pushed there in a while. I'll look into this quick but might need to raise this with someone else on the Team."

@mananjhaveri
Copy link

Hello @mchtey @isms
What's the latest with this PR?

I would also want a version bump for the ultralytics package so that I can experiment with yolov9. I would also like to experiment with onnx, so it would be helpful if we can add the onnxruntime package as well.

If there is a concern regarding ultralytics version bump, can we at least take forward this PR with just the addition of onnxruntime?

@mchtey
Copy link
Contributor Author

mchtey commented Mar 25, 2024

image

Hello, the update has been added. I wanted to let you know. :)

@isms
Copy link
Contributor

isms commented Mar 26, 2024

@mchtey Thanks for the update - just ran the build and it looks like we're good to go. Merging!

@isms isms merged commit 91a71fc into drivendataorg:main Mar 26, 2024
1 check passed
@mchtey
Copy link
Contributor Author

mchtey commented Mar 26, 2024

@isms thank you very much :)

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.

3 participants