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.

Sign up for GitHub

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

Jump to bottom

Support for W&B logger with VOC datasets #1525

Merged
merged 12 commits into from Nov 3, 2022
Merged

Support for W&B logger with VOC datasets #1525

merged 12 commits into from Nov 3, 2022

Conversation

manangoel99
Copy link
Contributor

This PR augments the current W&B logging infrastructure which worked with COCO style datasets to now also support logging and visualizing VOC datasets

@manangoel99 manangoel99 mentioned this pull request Sep 29, 2022
Closed
@manangoel99
Copy link
Contributor Author

Fixes #1412

@manangoel99
Copy link
Contributor Author

Hi @FateScript I was wondering if it was possible to get a review on this?

@FateScript
Copy link
Member

Hi @manangoel99 , thanks for reminding me of your pull request. There are so many PR in our repo and I will review them(including this) one by one tomorrow.
I'm busy with my work these days. Thanks for your patience!

FateScript
FateScript requested changes Oct 28, 2022
View reviewed changes
Copy link
Member

@FateScript FateScript left a comment

Choose a reason for hiding this comment

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

PTAL @manangoel99

yolox/exp/base_exp.py Outdated Show resolved Hide resolved
yolox/exp/yolox_base.py Outdated Show resolved Hide resolved
yolox/utils/logger.py Outdated Show resolved Hide resolved
yolox/utils/logger.py Outdated Show resolved Hide resolved
yolox/data/datasets/voc.py Outdated Show resolved Hide resolved
yolox/utils/logger.py Show resolved Hide resolved
yolox/utils/logger.py Outdated Show resolved Hide resolved
yolox/utils/logger.py Outdated Show resolved Hide resolved
yolox/utils/logger.py Outdated Show resolved Hide resolved
yolox/utils/logger.py Outdated Show resolved Hide resolved
@FateScript
Copy link
Member

@manangoel99 Any update?

@manangoel99
Copy link
Contributor Author

Hi @FateScript ! Thank you so much for the comprehensive review! Apologies for the delay. I got caught up with some other work.
Will be resolving the comments today.

@manangoel99
Copy link
Contributor Author

@FateScript I've added the suggested changes. Please have a look whenever you can.

@manangoel99
Copy link
Contributor Author

I'm not sure why the test is failing
Screenshot 2022-10-31 at 5 46 51 PM

@FateScript
Copy link
Member

FateScript commented Nov 1, 2022 •
edited

@manangoel99 The workflow failed because of updating of importlib-meta. Reference stackoverflow here, could you plz also update the ci.yaml file under .github/workflow and using a fixed version of importlib-metadata ?

@manangoel99
Copy link
Contributor Author

@FateScript I've set the version in ci.yml file

FateScript
FateScript requested changes Nov 2, 2022
View reviewed changes
Copy link
Member

@FateScript FateScript left a comment

Choose a reason for hiding this comment

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

@manangoel99 PTAL, some of your code still not change as I suggested.

yolox/exp/base_exp.py Outdated Show resolved Hide resolved
yolox/exp/yolox_base.py Outdated Show resolved Hide resolved
@manangoel99
Copy link
Contributor Author

@FateScript Apologies for this! I've fixed it now and pushed it.

@FateScript
Copy link
Member

LGTM, thanks for your contribution @manangoel99

@FateScript FateScript merged commit 24fd19f into Megvii-BaseDetection:main Nov 3, 2022
Githubinme pushed a commit to Githubinme/YOLOX that referenced this pull request Jun 6, 2023
@manangoel99
feat(logger): W&B logger with VOC datasets (Megvii-BaseDetection#1525)
6724078 
feat(logger): W&B logger with VOC datasets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FateScript FateScript FateScript approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@manangoel99 @FateScript