[tests/TF/build] enable missing classification onnx tests and set tensorflow lower bound to 2.11 by felixT2K · Pull Request #1182 · mindee/doctr (original) (raw)
This PR:
- upgrades the minor version of tf2onnx to >=1.14 (which supports TF 2.11)
- upgrades the minor version of tensorflow to >= 2.11 (first version where the issue seems to be fixed)
- enables tests
Codecov Report
Merging #1182 (39ba21a) into main (70ff4fd) will not change coverage.
The diff coverage isn/a.
❗ Current head 39ba21a differs from pull request most recent head de5cf13. Consider uploading reports for the commit de5cf13 to get more accurate results
@@ Coverage Diff @@ ## main #1182 +/- ##
Coverage 95.01% 95.01%
Files 149 149
Lines 6417 6417
Hits 6097 6097
Misses 320 320
| Flag | Coverage Δ |
|---|---|
| unittests | 95.01% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes
@frgfm Are you fine with increasing the minimum versions to fix this ? I think it would be also the time to drop py3.6 and 3.7 support and focus on 3.9 and 3.10 (especially CI) with >=3.8,<4
@felixdittrich92 Should it fix:
tensorflow.python.framework.errors_impl.InvalidArgumentError: {{function_node __wrapped__Conv2DBackpropInput_device_/job:localhost/replica:0/task:0/device:CPU:0}} Gradients for grouped convolutions are not supported on CPU. Please file a feature request if you run into this issue. Computed input depth 576 doesn't match filter input depth 1 [Op:Conv2DBackpropInput]
?
@felixdittrich92 Should it fix:
tensorflow.python.framework.errors_impl.InvalidArgumentError: {{function_node __wrapped__Conv2DBackpropInput_device_/job:localhost/replica:0/task:0/device:CPU:0}} Gradients for grouped convolutions are not supported on CPU. Please file a feature request if you run into this issue. Computed input depth 576 doesn't match filter input depth 1 [Op:Conv2DBackpropInput]?
@odulcy-mindee
Unfortunately no, the problem should actually be fixed since TF 2.9. I also tested it locally and it persists. However, only during training on the CPU (should normally not the case 😅 ). Training on the GPU and inference on the CPU and GPU runs without any problems.
But this PR fixes the failing onnx exports for TF mobilenet. After this we have finally all models available for exporting into onnx format (up to the logits)
The idea to drop py3.6 and py3.7 support is only to keep the repo + CI tests up to date
@odulcy-mindee fine with the changes in this PR ? I opened #1183 for the grouped conv issue 👍🏼
@frgfm Are you fine with increasing the minimum versions to fix this ? I think it would be also the time to drop py3.6 and 3.7 support and focus on 3.9 and 3.10 (especially CI) with >=3.8,<4
Regarding the version specifiers, yes that's alright but for future debugging purposes, I suggest updating the PR description to specify which new features we want to get out of those recent releases (easier to track down problems in a specific version later on).
About the python version, let me open a quick PR to check that (Python version upgrade can sometimes be tricky when the project on large platform-specific builds like DL frameworks) 👍
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again! Only a few suggestions for later debugging 👌
[tests/TF] enable missing classification onnx tests [tests/TF/build] enable missing classification onnx tests and set tensorflow lower bound to 2.11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})