Skip to content

Add COCO model for RKNN#2026

Merged
Gold856 merged 2 commits intoPhotonVision:mainfrom
samfreund:new-default-rknn
Aug 4, 2025
Merged

Add COCO model for RKNN#2026
Gold856 merged 2 commits intoPhotonVision:mainfrom
samfreund:new-default-rknn

Conversation

@samfreund
Copy link
Member

@samfreund samfreund commented Aug 3, 2025

Description

This adds the RKNN model trained on the COCO dataset as one of the models shipped with PV. This model is fairly general, and has been trained to identify a number of objects, including people, animals, cars, and more. This model is meant for teams to test object detection, particularly for teams who might not have access to the game elements that our other models are trained on.

Meta

Merge checklist:

  • Pull Request title is short, imperative summary of proposed changes
  • The description documents the what and why
  • If this PR changes behavior or adds a feature, user documentation is updated
  • If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly
  • If this PR touches configuration, this is backwards compatible with settings back to v2024.3.1
  • If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated
  • If this PR addresses a bug, a regression test for it is added

@samfreund samfreund requested a review from a team as a code owner August 3, 2025 22:38
@crschardt
Copy link
Contributor

A couple questions:

  1. Will this model require maintenance/upgrades over time?
  2. Will we need a different version for the RubikPi, or can it use this RKNN model?

And, since the proposal is for this to ship as part of PV, two requests:

  1. Can you add unit tests to confirm that it is working?
  2. What documentation should be included for users?

@samfreund
Copy link
Member Author

samfreund commented Aug 3, 2025

  1. Will this model require maintenance/upgrades over time?

No, it shouldn't need updates over time unless we happen to change something about the way we do RKNN.

  1. Will we need a different version for the RubikPi, or can it use this RKNN model?

Yes, we'll need a different version for Rubik, as the Rubik Pi uses a different format for models.

  1. Can you add unit tests to confirm that it is working?

Maybe? It depends on what you want me to test.

  1. What documentation should be included for users?

Hmm, I hadn't thought about that. I'll explain it under the OD section, and I think that should cover it?

@crschardt
Copy link
Contributor

  1. Can you add unit tests to confirm that it is working?

Maybe? It depends on what you want me to test.

It would be good to confirm that the model is working against one or more test images, but I'm not sure if that can only be done on an OrangePi, or if it can be run as part of the CI tests on GitHub.

@samfreund
Copy link
Member Author

  1. Can you add unit tests to confirm that it is working?

Maybe? It depends on what you want me to test.

It would be good to confirm that the model is working against one or more test images, but I'm not sure if that can only be done on an OrangePi, or if it can be run as part of the CI tests on GitHub.

Unfortunately, it can only be tested on opi unless we want to take the trouble of installing all the RKNN-toolkit libraries in CI. I don't think that's terribly feasible.

@crschardt
Copy link
Contributor

Unfortunately, it can only be tested on opi unless we want to take the trouble of installing all the RKNN-toolkit libraries in CI. I don't think that's terribly feasible.

That's what I was worried about. So, we rely on the scream test to know if it's not working. :(

@samfreund
Copy link
Member Author

Unfortunately, it can only be tested on opi unless we want to take the trouble of installing all the RKNN-toolkit libraries in CI. I don't think that's terribly feasible.

That's what I was worried about. So, we rely on the scream test to know if it's not working. :(

Essentially, yes 😢 I will likely test these models in the test matrix and before we cut a release, but yea.

Copy link
Member

@Gold856 Gold856 left a comment

Choose a reason for hiding this comment

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

LGTM besides the concerns of not being able to test this

Copy link
Member

@Gold856 Gold856 left a comment

Choose a reason for hiding this comment

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

Actually, we should document this.

@samfreund samfreund requested a review from Gold856 August 4, 2025 04:03
@samfreund samfreund added enhancement New feature or request backend Things relating to photon-core and photon-server labels Aug 4, 2025
@Gold856 Gold856 enabled auto-merge (squash) August 4, 2025 05:13
@Gold856 Gold856 merged commit 7531238 into PhotonVision:main Aug 4, 2025
42 checks passed
Gold856 added a commit to Gold856/photonvision that referenced this pull request Aug 4, 2025
Gold856 added a commit that referenced this pull request Aug 4, 2025
This reverts commit 7531238.

## Description

The COCO dataset contains images that use the NC and/or ND variants of
the CC license, and distributing a model based on that dataset is most
likely a violation of licenses. Additionally, the model is licensed under AGPL,
which might be a concern for PhotonVision, and at a minimum, there's no
license file bundled with the model right now.

## Meta

Merge checklist:
- [x] Pull Request title is [short, imperative
summary](https://cbea.ms/git-commit/) of proposed changes
- [x] The description documents the _what_ and _why_
- [x] If this PR changes behavior or adds a feature, user documentation
is updated
- [ ] If this PR touches photon-serde, all messages have been
regenerated and hashes have not changed unexpectedly
- [ ] If this PR touches configuration, this is backwards compatible
with settings back to v2024.3.1
- [ ] If this PR touches pipeline settings or anything related to data
exchange, the frontend typing is updated
- [ ] If this PR addresses a bug, a regression test for it is added
@samfreund samfreund mentioned this pull request Aug 5, 2025
7 tasks
Gold856 pushed a commit that referenced this pull request Aug 8, 2025
## Description

See #2026 for the previous iteration of this PR.

This adds the RKNN model trained on the COCO dataset as one of the
models shipped with PV. This model is fairly general, and has been
trained to identify a number of objects, including people, animals,
cars, and more. This model is meant for teams to test object detection,
particularly for teams who might not have access to the game elements
that our other models are trained on.

It additionally acknowledges Ultralytics for the model, and includes the
AGPL copyleft license.

## Meta

Merge checklist:
- [x] Pull Request title is [short, imperative
summary](https://cbea.ms/git-commit/) of proposed changes
- [x] The description documents the _what_ and _why_
- [x] If this PR changes behavior or adds a feature, user documentation
is updated
- [x] If this PR touches photon-serde, all messages have been
regenerated and hashes have not changed unexpectedly
- [x] If this PR touches configuration, this is backwards compatible
with settings back to v2024.3.1
- [x] If this PR touches pipeline settings or anything related to data
exchange, the frontend typing is updated
- [x] If this PR addresses a bug, a regression test for it is added
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Things relating to photon-core and photon-server enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants