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

[NEW PR] Dedicated WandbLogger for MMDetection #7459

Merged
merged 23 commits into from
May 27, 2022

Conversation

ayulockin
Copy link
Contributor

This PR is opened to sign the CLA agreement. The previous PR #7413 is closed in favor of this. You can check that PR for initial discussion.

The details of the feature can be found in this issue #7391.

@jbwang1997 jbwang1997 self-requested a review March 19, 2022 07:27
@codecov
Copy link

codecov bot commented Mar 19, 2022

Codecov Report

Merging #7459 (27f9bc3) into dev (857ac4e) will decrease coverage by 0.03%.
The diff coverage is 16.25%.

@@            Coverage Diff             @@
##              dev    #7459      +/-   ##
==========================================
- Coverage   64.76%   64.72%   -0.04%     
==========================================
  Files         351      358       +7     
  Lines       28465    29092     +627     
  Branches     4807     4940     +133     
==========================================
+ Hits        18436    18831     +395     
- Misses       9054     9261     +207     
- Partials      975     1000      +25     
Flag Coverage Δ
unittests 64.70% <16.25%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmdet/core/hook/wandblogger_hook.py 14.46% <14.46%> (ø)
mmdet/core/evaluation/eval_hooks.py 82.71% <100.00%> (+0.89%) ⬆️
mmdet/core/hook/__init__.py 100.00% <100.00%> (ø)
mmdet/utils/misc.py 61.53% <0.00%> (-2.57%) ⬇️
mmdet/models/roi_heads/mask_heads/maskiou_head.py 87.35% <0.00%> (-2.30%) ⬇️
mmdet/core/bbox/assigners/atss_assigner.py 92.04% <0.00%> (-1.47%) ⬇️
mmdet/core/evaluation/mean_ap.py 70.08% <0.00%> (-1.11%) ⬇️
mmdet/apis/train.py 13.68% <0.00%> (-0.15%) ⬇️
mmdet/core/__init__.py 100.00% <0.00%> (ø)
mmdet/utils/__init__.py 100.00% <0.00%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 857ac4e...27f9bc3. Read the comment docs.

@ayulockin
Copy link
Contributor Author

Hey all, the codecov workflow is failing. I guess it's because a unit test is not covering the added lines of code. I can write the unit test but would love to get some direction on this. Can you point me to a document/link or example tests based on which I can write the test?

@ZwwWayne ZwwWayne added this to In progress in New Features via automation Mar 27, 2022
@ayulockin
Copy link
Contributor Author

Hey @ZwwWayne, @RangiLyu, @hhaAndroid, @jbwang1997 would love to get some feedback/review on this PR. Apologies to ping again. Just wanted to confirm that there's nothing remaining from my side on this. Thanks!

@ZwwWayne ZwwWayne changed the base branch from master to dev March 30, 2022 03:28
@jbwang1997
Copy link
Collaborator

Sorry for the later reply. We are currently busy with the vision release. We have planned to merge this pr in as soon as the next vision and I leave some comments below. Thank you so much for your valuable contribution.

@hhaAndroid
Copy link
Collaborator

@ayulockin I will update with you.

@hhaAndroid
Copy link
Collaborator

hhaAndroid commented Apr 1, 2022

@ayulockin Hello

  1. mmcv's wandbhook already supports log eval metrics, can your PR delete this part of the code?
  2. I see that mask visualization is not supported yet.

@ayulockin
Copy link
Contributor Author

  1. mmcv's wandbhook already supports log eval metrics, can your PR delete this part of the code?

Yes I can remove this part.

  1. I see that mask visualization is not supported yet.

Yes, wandb doesn't support instance segmentation as of yet.

@hhaAndroid
Copy link
Collaborator

I see it's supported https://docs.wandb.ai/ref/python/data-types/imagemask. Each bbox comes with a mask_data to complete instance segmentation visualization

@julled
Copy link

julled commented Apr 2, 2022

@ayulockin i really like this PR, thx for implementing all the valuable functions!

I am not a super duper expert in mlflow but from what i see from your code it might be that the functions you introduce in your PR are with minor changes also nicely usable for a smiliar mlflow hook.

Do you there might be some generic functions to be put in a kind of "common" library to use e.g. for a similar mlflow hook extension?

@ayulockin
Copy link
Contributor Author

@ayulockin i really like this PR, thx for implementing all the valuable functions!

I am not a super duper expert in mlflow but from what i see from your code it might be that the functions you introduce in your PR are with minor changes also nicely usable for a smiliar mlflow hook.

Do you there might be some generic functions to be put in a kind of "common" library to use e.g. for a similar mlflow hook extension?

Hey @julled, glad you liked the work.

I have not used MLFlow myself so can't say if we can abstract away some of the class methods as a utility that can be used for the MLFlow hook. The MMDetWandbHook introduced in this PR inherits WandbLoggerHook. If you wanna implement an extended hook for MLFlow there is a MlflowLoggerHook that you can inherit.

Also, the hook that this PR is introducing is adding functionalities that are specific to W&B.

I hope this helps.

@ayulockin
Copy link
Contributor Author

I see it's supported https://docs.wandb.ai/ref/python/data-types/imagemask. Each bbox comes with a mask_data to complete instance segmentation visualization

Hey @hhaAndroid, yes logging mask_data is supported by W&B but the issue is that instance segmentation visualization is not supported by the UI properly. If the mask data is for semantic segmentation then the UI can render it nicely. Since MMDetection doesn't support semantic segmentation, I think the feature will not be very useful now.

I can add the feature once instance segmentation visualization is supported by W&B. Thoughts?

@HJoonKwon
Copy link
Contributor

HJoonKwon commented May 12, 2022

@ayulockin Thank you so much!! This PR was very useful for my project, and saved a lot of time. Actually, somehow after_train_iter was blocking the training/logging process so I just commented it out, and everything works fine. Hope it'd be useful information for you. (self.by_epoch was true in my case)

@ayulockin
Copy link
Contributor Author

Hey @HJoonKwon glad you found it useful. I am curious to know what kind of blocker did you face with after_train_iter method present in the code. Can you share any error message or log? Would love to debug it since I am not facing the same on my end.

@hhaAndroid
Copy link
Collaborator

@ayulockin Hello! Do you have any progress?

@ayulockin
Copy link
Contributor Author

Do you have resources for training and then release the wandb log?

Hey do you need config for training using IterBasedRunner?

@ayulockin
Copy link
Contributor Author

@ayulockin Hello! Do you have any progress?

Hey @hhaAndroid I was able to add the config saving feature the way you suggested. I was also able to log the config as W&B config and it feels like a good feature enhancement for the logger. Please let me know if it looks good or now.

I will also provide a colab notebook that can act as a training resource.

@hhaAndroid
Copy link
Collaborator

colab notebook

it's very good.

@ayulockin
Copy link
Contributor Author

Hey @hhaAndroid here's the colab notebook for training using MMDetWandbLogger: https://github.com/ayulockin/myDLexperiments/blob/master/Train_an_Object_Detection%2BSemantic_Segmentation_Model_with_MMDetection_and_W%26B.ipynb

Please let me know if you face any issue while running it.

I have shared it from my GitHub repo. If you find it useful and educational for community, I can add it to README or docs.

New Features automation moved this from In progress to Reviewer approved May 27, 2022
@ZwwWayne
Copy link
Collaborator

Hey @hhaAndroid here's the colab notebook for training using MMDetWandbLogger: https://github.com/ayulockin/myDLexperiments/blob/master/Train_an_Object_Detection%2BSemantic_Segmentation_Model_with_MMDetection_and_W%26B.ipynb

Please let me know if you face any issue while running it.

I have shared it from my GitHub repo. If you find it useful and educational for community, I can add it to README or docs.

Hi @ayulockin ,
Thanks for your kind contribution. The colab tutorial is indeed useful to us. Could you provide a sharable link in google colab like 'https://colab.research.google.com/drive/xxx'? This link can be add in the docstring of the MMDetWandbHook added in this PR, and we can also add a new sentence here: https://github.com/open-mmlab/mmdetection#getting-started, like 'There are also a wonderful colab tutorial based Weights and Biases contributed by @ayulockin '

@ayulockin
Copy link
Contributor Author

Thanks for your kind comment @ZwwWayne. I will provide a drive link soon.

@ayulockin
Copy link
Contributor Author

Hey @ZwwWayne here's the colab link: https://colab.research.google.com/drive/1RCSXHZwDZvakFh3eo9RuNrJbCGqD0dru?usp=sharing

@ZwwWayne ZwwWayne merged commit b637c99 into open-mmlab:dev May 27, 2022
New Features automation moved this from Reviewer approved to Done May 27, 2022
@saulgoldsaga
Copy link

I had to add
if not any(masks): masks = None
after line 350 in wandblogger_hook.py
to ensure wandb didn't think I had any masks.

@saulgoldsaga
Copy link

Thanks @ayulockin for putting this together, it has saved me lots of time!
I was wondering if you had done any work on uploading training and validation image examples to wandb?
I thought I should ask before doing myself

@ayulockin
Copy link
Contributor Author

ayulockin commented May 31, 2022

I was wondering if you had done any work on uploading training and validation image examples to wandb?

Hey @saulgoldsaga, this PR adds this feature. If num_eval_images is greater than 0 it will log the validation images to W&B.

You can use the same logic for training images.

ZwwWayne added a commit that referenced this pull request Jul 18, 2022
* [Fix] Adjust the order of get_classes and FileClient. (#7276)

* delete -sv (#7277)

Co-authored-by: Wenwei Zhang <40779233+ZwwWayne@users.noreply.github.com>

* wandb-integration

* docstring

* lint

* log config+handle segmentation results

* remove val logging

* segmentation logging

* metadata logging improved + remove best metadata

* remove extra arg

* train.py conflict

* shuffle eval data+remove config

* wandb config file

* iter based logging + cleanup

* minor update

* minor update

* minor fix

* epoch and iter eval table

* save and log config

* Delete mask_rcnn_r50_fpn_1x_wandb_iter_coco.py

Co-authored-by: Wencheng Wu <41542251+274869388@users.noreply.github.com>
Co-authored-by: Yue Zhou <592267829@qq.com>
Co-authored-by: Wenwei Zhang <40779233+ZwwWayne@users.noreply.github.com>
ZwwWayne added a commit to ZwwWayne/mmdetection that referenced this pull request Jul 19, 2022
* [Fix] Adjust the order of get_classes and FileClient. (open-mmlab#7276)

* delete -sv (open-mmlab#7277)

Co-authored-by: Wenwei Zhang <40779233+ZwwWayne@users.noreply.github.com>

* wandb-integration

* docstring

* lint

* log config+handle segmentation results

* remove val logging

* segmentation logging

* metadata logging improved + remove best metadata

* remove extra arg

* train.py conflict

* shuffle eval data+remove config

* wandb config file

* iter based logging + cleanup

* minor update

* minor update

* minor fix

* epoch and iter eval table

* save and log config

* Delete mask_rcnn_r50_fpn_1x_wandb_iter_coco.py

Co-authored-by: Wencheng Wu <41542251+274869388@users.noreply.github.com>
Co-authored-by: Yue Zhou <592267829@qq.com>
Co-authored-by: Wenwei Zhang <40779233+ZwwWayne@users.noreply.github.com>
ZwwWayne added a commit to ZwwWayne/mmdetection that referenced this pull request Jul 19, 2022
* [Fix] Adjust the order of get_classes and FileClient. (open-mmlab#7276)

* delete -sv (open-mmlab#7277)

Co-authored-by: Wenwei Zhang <40779233+ZwwWayne@users.noreply.github.com>

* wandb-integration

* docstring

* lint

* log config+handle segmentation results

* remove val logging

* segmentation logging

* metadata logging improved + remove best metadata

* remove extra arg

* train.py conflict

* shuffle eval data+remove config

* wandb config file

* iter based logging + cleanup

* minor update

* minor update

* minor fix

* epoch and iter eval table

* save and log config

* Delete mask_rcnn_r50_fpn_1x_wandb_iter_coco.py

Co-authored-by: Wencheng Wu <41542251+274869388@users.noreply.github.com>
Co-authored-by: Yue Zhou <592267829@qq.com>
Co-authored-by: Wenwei Zhang <40779233+ZwwWayne@users.noreply.github.com>
SakiRinn pushed a commit to SakiRinn/mmdetection-locount that referenced this pull request Mar 17, 2023
* [Fix] Adjust the order of get_classes and FileClient. (open-mmlab#7276)

* delete -sv (open-mmlab#7277)

Co-authored-by: Wenwei Zhang <40779233+ZwwWayne@users.noreply.github.com>

* wandb-integration

* docstring

* lint

* log config+handle segmentation results

* remove val logging

* segmentation logging

* metadata logging improved + remove best metadata

* remove extra arg

* train.py conflict

* shuffle eval data+remove config

* wandb config file

* iter based logging + cleanup

* minor update

* minor update

* minor fix

* epoch and iter eval table

* save and log config

* Delete mask_rcnn_r50_fpn_1x_wandb_iter_coco.py

Co-authored-by: Wencheng Wu <41542251+274869388@users.noreply.github.com>
Co-authored-by: Yue Zhou <592267829@qq.com>
Co-authored-by: Wenwei Zhang <40779233+ZwwWayne@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

9 participants