-
Notifications
You must be signed in to change notification settings - Fork 605
Quick fix: Add user conf to yolo reference implementation #2446
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
Open
pgmpablo157321
wants to merge
9
commits into
master
Choose a base branch
from
yolo_fix
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+110
−22
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
008f6ec
Quick fix: Add user conf to yolo reference implementation
pgmpablo157321 4b25029
Add model_name to yolo implementation
pgmpablo157321 ef62852
[Automated Commit] Format Codebase
github-actions[bot] c4e2b0e
Fix default name
pgmpablo157321 d34e8da
Fix logging for YOLO benchmark (#2450)
anandhu-eng 97d502c
[Automated Commit] Format Codebase
github-actions[bot] 09537da
Updates for YOLO (#2452)
anandhu-eng 5a61d79
Update models from submission checker constants (#2464)
anandhu-eng 1463483
Generate final report: Update filter scenarios for version 6.0 (#2465)
anandhu-eng File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 5000 when the dataset size is 1513?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it was present in submission checker already, just copied down here. @manpreetssokhi , would it be fine if we bring it down to 1513?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
500 is used here: https://github.com/mlcommons/inference/blob/master/vision/classification_and_detection/yolo/yolo_loadgen.py#L150
The count should be such that the memory needed to load that much dataset should ideally be above a few MBs (> L3 size) but still run on edge systems (not above say 256MB or so)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in yolo_loadgen.py I had tried both 1525 and 500. I believe that the yolo.*.performance_sample_count_override = 5000 comes from retinanet being 5000. I am open to adjusting it to meet the MLC rules. I think it might be best to keep 1525 across:
https://github.com/mlcommons/inference/blob/master/vision/classification_and_detection/yolo/yolo_loadgen.py#L150
and
https://github.com/mlcommons/inference/pull/2446/files/97d502c84c1e574a6ca4f5a1d6bf5f877ffe1ad6..09537da212a780471f563fc63ff9d1f6edea4910#diff-60781b468b10ba9bf59f52a09114c63209a92f299bf957299a055a99900a35c8
what do you both think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
retinanet performance_sample_count is actually 64. If the image size is the same as in retinanet, we can use 64 itself for yolo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, then in that case we can go with 64 for yolo here. Does this overwrite the min of 500 performance_sample_count in the base implementation I have? I am trying to understand the difference and more broadly the different components in the repo. My intention behind the higher numbers of 500 and 1525 was to make sure we meet the minimum 10 min run.