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

Fix bugs when ploting loss curve by analyze_logs.py #8944

Merged
merged 2 commits into from
Oct 18, 2022

Conversation

sltlls
Copy link
Contributor

@sltlls sltlls commented Oct 5, 2022

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

When ploting loss_cls of some run using analyze_logs.py, the numbers of xs and ys in function plot_curve(line 83,84) are not the same! Therefore causing error when running plt.plot().
After debug, I found that this problem was caused by the quantity mismatch of values of key 'step' and key 'loss_cls' in log_dicts returned by function load_json_logs. And this problem occured because validation lines in json log file producing after training also contains key 'step', causing the number of key 'step' in log_dicts 12 more than that in key 'loss_cls'.

Modification

Simply add val_flag variable to judge whether current line is validation line, if it is, do not add the step info into the log_dicts.

BC-breaking (Optional)

Does the modification introduce changes that break the backward-compatibility of the downstream repos?
no

@CLAassistant
Copy link

CLAassistant commented Oct 5, 2022

CLA assistant check
All committers have signed the CLA.

@jbwang1997 jbwang1997 changed the base branch from 3.x to dev-3.x October 6, 2022 09:30
@sltlls
Copy link
Contributor Author

sltlls commented Oct 6, 2022

do I need to recommit the PR after installing pre-commit in my local repository?

@wanghonglie
Copy link

do I need to recommit the PR after installing pre-commit in my local repository?
Yes, please push it after updating.

@sltlls
Copy link
Contributor Author

sltlls commented Oct 7, 2022

Ok, I have already pushed it after updating my local repository

@ZwwWayne ZwwWayne requested a review from RangiLyu October 8, 2022 02:41
@ZwwWayne ZwwWayne assigned wanghonglie and unassigned jbwang1997 Oct 8, 2022
@ZwwWayne ZwwWayne added this to the 3.0.0rc2 milestone Oct 8, 2022
@RangiLyu
Copy link
Member

Ok, I have already pushed it after updating my local repository

Seems you did not push to GitHub.

@ZwwWayne ZwwWayne merged commit babf9c2 into open-mmlab:dev-3.x Oct 18, 2022
MambaWong pushed a commit to MambaWong/mmdetection-1 that referenced this pull request Oct 20, 2022
MambaWong pushed a commit to MambaWong/mmdetection-1 that referenced this pull request Oct 21, 2022
MambaWong pushed a commit to MambaWong/mmdetection-1 that referenced this pull request Oct 22, 2022
MambaWong pushed a commit to MambaWong/mmdetection-1 that referenced this pull request Oct 22, 2022
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

6 participants