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 yolov10 detection node #8753

Open
3 of 7 tasks
storrrrrrrrm opened this issue Sep 4, 2024 · 14 comments
Open
3 of 7 tasks

add yolov10 detection node #8753

storrrrrrrrm opened this issue Sep 4, 2024 · 14 comments
Assignees
Labels
component:perception Advanced sensor data processing and environment understanding. (auto-assigned)

Comments

@storrrrrrrrm
Copy link
Contributor

Checklist

  • I've read the contribution guidelines.
  • I've searched other issues and no duplicate issues were found.
  • I've agreed with the maintainers that I can plan this task.

Description

add a node which use latest yolo10 model to do detection

Purpose

yolo10 is faster and has higher AP on coco compared to previous yolo series models. it may help to imporove detection performance

Possible approaches

https://github.com/THU-MIG/yolov10 refer to above python code, we can implement a node that uses tensorrt for inference.

Definition of done

  • c++ code,use trt convert onnx to engine
  • add preprocess
  • add postprocess
  • test on bag
@storrrrrrrrm storrrrrrrrm self-assigned this Sep 4, 2024
@amadeuszsz amadeuszsz added the component:perception Advanced sensor data processing and environment understanding. (auto-assigned) label Sep 4, 2024
@mitsudome-r
Copy link
Member

@storrrrrrrrm
Thanks for creating the issue. @kminoda was worried about the license of YOLO v10. It seems like it is using AGPL license, which is copy left license. Do you have any idea how you plan to merge this with Autoware at the moment? (e.g., keeping it as a separate the repository)

@xmfcx
Copy link
Contributor

xmfcx commented Sep 4, 2024

If the inference code could be reused from existing Autoware, it shouldn't matter.

https://github.com/THU-MIG/yolov10/blob/main/docs/en/integrations/tensorrt.md

We will only use the model that is exported to TensorRT format.

But if we are going to use the TensorRT inference codes from their repository, we cannot put it under Autoware.Universe.

Then we can create a sub-repo under autowarefoundation and add it to autoware.repos file.

@storrrrrrrrm
Copy link
Contributor Author

@storrrrrrrrm Thanks for creating the issue. @kminoda was worried about the license of YOLO v10. It seems like it is using AGPL license, which is copy left license. Do you have any idea how you plan to merge this with Autoware at the moment? (e.g., keeping it as a separate the repository)

don't worry, I will just refer to the code logic, and I will implement new node code based on the existing code of autoware.

@liuXinGangChina liuXinGangChina moved this to In Progress in Autoware Labs Sep 10, 2024
@storrrrrrrrm
Copy link
Contributor Author

Progress update: I haven’t had much time to work on this task recently. For now the basic code framework has been built and can be compiled, but the specific logic code has not been added yet.
image

@storrrrrrrrm
Copy link
Contributor Author

Progress update: single image infer done.
image
image

@storrrrrrrrm
Copy link
Contributor Author

Progress Update:
image

play a ros bag,this node would output a DetectedObjectsWithFeature. but when i watch output image on rviz,there is sth wrong with box position om img. to be debug.

@storrrrrrrrm
Copy link
Contributor Author

Progress Update: image

play a ros bag,this node would output a DetectedObjectsWithFeature. but when i watch output image on rviz,there is sth wrong with box position om img. to be debug.

solved this.
Peek 2024-10-30 17-49

@kminoda
Copy link
Contributor

kminoda commented Nov 18, 2024

@storrrrrrrrm (cc: @mitsudome-r @xmfcx )
Hi, one of my colleague found this issue comment from ultralytics: ultralytics/ultralytics#2129 (comment).
I understand that YOLOv10 is not from ultralytics but some researchers from Tsinghua University, but given that the similar library's maintainer mentions that AGPL license is applicable not only the inference code but also the model weight, it would be better to ask the author of YOLOv10 about the license somehow to clarify the legal risks. What do you think?

@liuXinGangChina
Copy link

Morning kminoda san @kminoda

Thanks for your remind,I check the original repo of yolo -v10 and found a open discussion about license of the mode discussion link 。 It looks that the author did not give a response yet

since the tensorrt-yolov10-node code is totally refactored based on tire4’s tensorrt-yolox , i believe it should follow tire4‘s license like shin san suggested (apache 2.0) discusison link is here

Looking forward to your further comment

Have a nice day!

Xingang

@kminoda
Copy link
Contributor

kminoda commented Nov 21, 2024

@liuXinGangChina Thank you for the response.

since the tensorrt-yolov10-node code is totally refactored based on tire4’s tensorrt-yolox , i believe it should follow tire4‘s license like shin san suggested (apache 2.0) discusison link is #9198 (comment)

Yes, I understand this, but my point is that there might be a legal risk if we include the trained model from YOLOv10 repository (which would also be AGPL license) into autoware.universe. Do you have any plans to resolve this issue? Some solutions I can come up with are

  1. Retrain the model from scratch on your end and use that weight (as you've commented in the PR)
  2. Keep the YOLOv10 separate from the autoware.universe repository for now (similar to what Mitsudome-san suggested here)
  3. etc...

@mitsudome-r @xmfcx May I hear your thoughts?

@liuXinGangChina
Copy link

@liuXinGangChina Thank you for the response.

since the tensorrt-yolov10-node code is totally refactored based on tire4’s tensorrt-yolox , i believe it should follow tire4‘s license like shin san suggested (apache 2.0) discusison link is #9198 (comment)

Yes, I understand this, but my point is that there might be a legal risk if we include the trained model from YOLOv10 repository (which would also be AGPL license) into autoware.universe. Do you have any plans to resolve this issue? Some solutions I can come up with are

1. Retrain the model from scratch on your end and use that weight (as you've commented in the PR)

2. Keep the YOLOv10 separate from the autoware.universe repository for now (similar to what Mitsudome-san suggested [here](https://github.com/autowarefoundation/autoware.universe/issues/8753#issuecomment-2329039888))

3. etc...

@mitsudome-r @xmfcx May I hear your thoughts?

Thanks for your suggestion, we would prefer solution 2 (keep the repo seperate from autoware )

let's wait for @mitsudome-r and @xmfcx ‘s decision

have a nice day!

Lucas

@xmfcx
Copy link
Contributor

xmfcx commented Nov 25, 2024

As mentioned in this thread and the PR thread, the inference code is licensed under Apache 2.0, and the PR includes only this code.

The model and training code are licensed under AGPL-3.0.

We can proceed with merging the PR since it only includes the inference code, which aligns with our project's licensing requirements.

To ensure clarity and compliance, we should update the README to specify:

  1. The inference code is licensed under Apache 2.0.
  2. The model and training code are licensed under AGPL-3.0.

We should also briefly explain the AGPL-3.0 license, particularly its requirement to share the "training source code" when the model is deployed over a network. This will help users understand their responsibilities when using the repository.

Similar to: https://github.com/autowarefoundation/autoware.universe/tree/main/perception/autoware_lidar_centerpoint#legal-notice

I don't think we need to take urgent action on preparing a new repository for the time being. The model is not even uploaded to our servers yet to be distributed.

@kminoda @liuXinGangChina do you still have any questions or concerns?

@liuXinGangChina
Copy link

As mentioned in this thread and the PR thread, the inference code is licensed under Apache 2.0, and the PR includes only this code.

The model and training code are licensed under AGPL-3.0.

We can proceed with merging the PR since it only includes the inference code, which aligns with our project's licensing requirements.

To ensure clarity and compliance, we should update the README to specify:

1. The inference code is licensed under Apache 2.0.

2. The model and training code are licensed under AGPL-3.0.

We should also briefly explain the AGPL-3.0 license, particularly its requirement to share the "training source code" when the model is deployed over a network. This will help users understand their responsibilities when using the repository.

Similar to: https://github.com/autowarefoundation/autoware.universe/tree/main/perception/autoware_lidar_centerpoint#legal-notice

I don't think we need to take urgent action on preparing a new repository for the time being. The model is not even uploaded to our servers yet to be distributed.

@kminoda @liuXinGangChina do you still have any questions or concerns?

Thanks for your comment, Mr Fatih. @xmfcx

Totally agree with you, and what you suggest is what we trying todo in the begining.
we will update the readme soon to make the license issue clear to autoware developer.

Have a nice day, kminoda-san and Mr. Fatih

Xingang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:perception Advanced sensor data processing and environment understanding. (auto-assigned)
Projects
Status: In Progress
Development

No branches or pull requests

6 participants