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] sar_resnet31 TF + PT #1513

Merged
merged 3 commits into from
Mar 15, 2024
Merged

Conversation

felixdittrich92
Copy link
Contributor

This PR:

  • Fixed inference decoding
  • Add missing benchmark
  • Fixed half precision exception

NOTE:

  • with half precision it produces some gibberish looks like an overflow on decoder side (same problem as with vitstr_base) this needs some deeper insight but is not really critical -> other ticket
  • I made a small mistake while fixing the SAR architecture:
    I double used the lstmcell on decoder side in pytorch, this explains the small difference in model parameters: TF: 57.2 M --> PT: 55.4 M -> but it does not impact the result quality (as you can see in the benchmarks there is no drop) so i don't see any need to fix and retrain the pt model

Any feedback is welcome 🤗

Closes: #1411 #1441

@felixdittrich92 felixdittrich92 added type: bug Something isn't working topic: documentation Improvements or additions to documentation module: models Related to doctr.models framework: pytorch Related to PyTorch backend framework: tensorflow Related to TensorFlow backend topic: text recognition Related to the task of text recognition ext: docs Related to docs folder labels Mar 15, 2024
@felixdittrich92 felixdittrich92 added this to the 0.9.0 milestone Mar 15, 2024
@felixdittrich92 felixdittrich92 self-assigned this Mar 15, 2024
@felixdittrich92 felixdittrich92 linked an issue Mar 15, 2024 that may be closed by this pull request
Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.82%. Comparing base (709404e) to head (00a99b7).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1513      +/-   ##
==========================================
+ Coverage   95.80%   95.82%   +0.02%     
==========================================
  Files         166      166              
  Lines        7646     7645       -1     
==========================================
+ Hits         7325     7326       +1     
+ Misses        321      319       -2     
Flag Coverage Δ
unittests 95.82% <100.00%> (+0.02%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@felixdittrich92 felixdittrich92 merged commit c2b197d into mindee:main Mar 15, 2024
70 checks passed
@felixdittrich92 felixdittrich92 deleted the sar-test branch March 15, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext: docs Related to docs folder framework: pytorch Related to PyTorch backend framework: tensorflow Related to TensorFlow backend module: models Related to doctr.models topic: documentation Improvements or additions to documentation topic: text recognition Related to the task of text recognition type: bug Something isn't working
Projects
None yet
2 participants