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

Feature/sg 1488 yolo nas r integration model #2001

Conversation

BloodAxe
Copy link
Contributor

@BloodAxe BloodAxe commented May 23, 2024

This PR contains:

  • YoloNAS-R model
  • YoloNAS-R loss
  • Export notebook
  • License file
  • Pretrained weights

]


class AbstractOBBDetectionDecodingModule(nn.Module):
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep this entire interface if the only difference between it and AbstractObjectDetectionDecodingModule is the returned item from get_output_names?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed that in fde191e, thanks!

Copy link
Contributor

@shaydeci shaydeci left a comment

Choose a reason for hiding this comment

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

Looks great!
Apart from inline comments and ongoing review, some suggestionsfor the notebook:

  • Limitations part: maybe its best to put those at the top?
  • I didn't see any mentioning of TRT support, maybe I missed it (ctrl+f tensorrt / trt to verify myself).

Also

  • Work on albu support - I think this is a valuable feature. Maybe add a section in our Albu suport botebook for rotated boxes too ?

Copy link
Contributor

@shaydeci shaydeci left a comment

Choose a reason for hiding this comment

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

So the only comment I have left is https://github.com/Deci-AI/super-gradients/pull/2001/files#r1615586708

Didn't we say we can make it inherit from AbstractObjectDetectionDecodingModule?
Sorry if Im mixing some of our talks up...

@BloodAxe
Copy link
Contributor Author

BloodAxe commented Jun 3, 2024

So the only comment I have left is https://github.com/Deci-AI/super-gradients/pull/2001/files#r1615586708

Didn't we say we can make it inherit from AbstractObjectDetectionDecodingModule? Sorry if Im mixing some of our talks up...

That's correct. I forgot to add that, thanks for noticing. Fixed in fde191e

Copy link
Contributor

@shaydeci shaydeci left a comment

Choose a reason for hiding this comment

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

LGTM

@BloodAxe BloodAxe merged commit e9e48d5 into feature/SG-1488-YoloNAS-R-integration Jun 4, 2024
4 checks passed
@BloodAxe BloodAxe deleted the feature/SG-1488-YoloNAS-R-integration-model branch June 4, 2024 07:53
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.

2 participants