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 panoptic driving perception demo using YOLOPv2 #204

Merged
merged 4 commits into from Oct 26, 2022

Conversation

DiegoFernandezC
Copy link
Member

@DiegoFernandezC DiegoFernandezC commented Oct 13, 2022

This PR introduces a new demo using the YOLOPv2 model.

@DiegoFernandezC DiegoFernandezC force-pushed the feat/YOLOPv2-demo branch 3 times, most recently from 6ddf1bf to 928440f Compare October 13, 2022 19:03
@DiegoFernandezC DiegoFernandezC changed the title feat: YOLOPv2 demo YOLOPv2 demo Oct 14, 2022
@DiegoFernandezC DiegoFernandezC changed the title YOLOPv2 demo Add YOLOPv2 demo Oct 14, 2022
Copy link
Collaborator

@javiber javiber left a comment

Choose a reason for hiding this comment

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

Looks good, the video seems to perform worst than theirs but is actually much harder so it's expected in a sense.

skimming through the code I got the feeling that we included some unnecessary dependencies and utils, even some functionality that logs performance. I was expecting a minimal demo like the others but maybe we want to have a different approach in this one

demos/yolopv2/README.md Outdated Show resolved Hide resolved
demos/yolopv2/README.md Outdated Show resolved Hide resolved
@dekked
Copy link
Member

dekked commented Oct 20, 2022

Need to add this demo to the main README doc.

Copy link
Collaborator

@javiber javiber left a comment

Choose a reason for hiding this comment

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

Can we add some comments on our code that points to the original code in the yolopv2 repo?

The motivation is to separate the code we used from their repo from the one e had to add to make the demo work. This will help us in the future to have the lineage of the code, and know what we did vs what is yolopv2 code.
Of course, this request is only valid if the distinction is clear, if you had to copy a function from them and then modify it then just add a general comment explaining that the code is based on their repo.

# img = img * 0.5 + color_seg * 0.5
# img = img.astype(np.uint8)
# img = cv2.resize(img, (1280,720), interpolation=cv2.INTER_LINEAR)
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove unnecessary return and commented code in this function and the above one
(I'm not against commented code that might be useful for debugging or future improvements but in that case, add a comment explaining why is there)

@dekked dekked changed the title Add YOLOPv2 demo Add panoptic driving perception demo using YOLOPv2 Oct 21, 2022
@DiegoFernandezC DiegoFernandezC force-pushed the feat/YOLOPv2-demo branch 2 times, most recently from d286194 to cf8ac65 Compare October 24, 2022 14:45
@@ -12,6 +12,7 @@ COPY requirements.txt /demo/requirements.txt

RUN pip install -r /demo/requirements.txt

RUN pip install git+https://github.com/tryolabs/norfair.git@master#egg=norfair
# Install PyTorch from URL
Copy link
Member

Choose a reason for hiding this comment

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

This comment is not helpful.

Why are we installing this version of PyTorch?

Copy link
Member Author

Choose a reason for hiding this comment

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

We had problems with the versions of Torch used. I didn't see that the actions failed. It was solved using the ones from the docker image.

@DiegoFernandezC DiegoFernandezC merged commit f5873bd into master Oct 26, 2022
@DiegoFernandezC DiegoFernandezC deleted the feat/YOLOPv2-demo branch October 26, 2022 19:47
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.

None yet

3 participants