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: deprecated clipping + removed 'None' metrics #432

Merged
merged 3 commits into from
Aug 26, 2021
Merged

Conversation

charlesmindee
Copy link
Collaborator

This PR fixes:

  • a deprecation warning in both pytorch training scripts with gradient clipping function.
  • a bug occurring when no boxes at all are found (edge case): the current LocalizationMetric gives back a None for both precision & IoU and this is blowing up the training script because metrics can't be logged properly.

Any feedback is welcome!

@charlesmindee charlesmindee added type: bug Something isn't working framework: pytorch Related to PyTorch backend labels Aug 25, 2021
@charlesmindee charlesmindee added this to the 0.3.1 milestone Aug 25, 2021
@charlesmindee charlesmindee self-assigned this Aug 25, 2021
@codecov
Copy link

codecov bot commented Aug 25, 2021

Codecov Report

Merging #432 (b93b5c2) into main (0dded4e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #432   +/-   ##
=======================================
  Coverage   95.84%   95.84%           
=======================================
  Files          96       96           
  Lines        3948     3948           
=======================================
  Hits         3784     3784           
  Misses        164      164           
Flag Coverage Δ
unittests 95.84% <ø> (ø)

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

Impacted Files Coverage Δ
doctr/models/recognition/crnn/pytorch.py 98.03% <0.00%> (ø)

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 0dded4e...b93b5c2. Read the comment docs.

Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
Agree with the inplace clipping, but I added a comment for the metric

doctr/utils/metrics.py Outdated Show resolved Hide resolved
Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

Thanks for the edit, I just added a small suggestion for the log message 👌

references/detection/train_pytorch.py Outdated Show resolved Hide resolved
references/detection/train_tensorflow.py Outdated Show resolved Hide resolved
Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

Thanks!

@charlesmindee charlesmindee merged commit 0676769 into main Aug 26, 2021
@charlesmindee charlesmindee deleted the train branch August 26, 2021 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework: pytorch Related to PyTorch backend type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants