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.

Sign up for GitHub

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

Jump to bottom

Feature/1124 Added Pytorch Lightning Test support for all models #1125

Merged
merged 20 commits into from
Jul 26, 2022
Merged

Feature/1124 Added Pytorch Lightning Test support for all models #1125

merged 20 commits into from
Jul 26, 2022

Conversation

AlexandreBrown
Copy link
Contributor

@AlexandreBrown AlexandreBrown commented Jul 20, 2022 •
edited
Loading

This closes #1124

  • Update models PL model adapter
  • Unit Tests
  • Update notebook documentation
  • End-to-end test in notebook
    image

@codecov
Copy link

codecov bot commented Jul 20, 2022 •
edited
Loading

Codecov Report

Merging #1125 (f3990a0) into master (356d29b) will increase coverage by 0.30%.
The diff coverage is 98.00%.

❗ Current head f3990a0 differs from pull request most recent head 3ee0e54. Consider uploading reports for the commit 3ee0e54 to get more accurate results

@@            Coverage Diff             @@
##           master    #1125      +/-   ##
==========================================
+ Coverage   85.48%   85.78%   +0.30%     
==========================================
  Files         305      305              
  Lines        6675     6697      +22     
==========================================
+ Hits         5706     5745      +39     
+ Misses        969      952      -17
Flag Coverage Δ
unittests 85.78% <98.00%> (+0.30%) ⬆️

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

Impacted Files Coverage Δ
...odels/ross/efficientdet/lightning/model_adapter.py 97.43% <93.75%> (+60.76%) ⬆️
icevision/models/mmdet/lightning/model_adapter.py 97.43% <100.00%> (+0.21%) ⬆️
...sion/models/torchvision/lightning_model_adapter.py 100.00% <100.00%> (ø)
...dels/ultralytics/yolov5/lightning/model_adapter.py 100.00% <100.00%> (ø)
icevision/__init__.py 86.66% <0.00%> (-13.34%) ⬇️
icevision/models/ross/efficientdet/loss_fn.py 100.00% <0.00%> (+25.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@AlexandreBrown
Copy link
Contributor Author

AlexandreBrown commented Jul 21, 2022 •
edited
Loading

I added some more unit tests to test the on_test_epoch_end, on_validation_epoch_end and the Trainer.validate.

If someone can re-run the github workflows I would appreciate it (I'm a first-time contributor so I cannot trigger it).

@AlexandreBrown AlexandreBrown mentioned this pull request Jul 22, 2022
Open
@AlexandreBrown
Copy link
Contributor Author

AlexandreBrown commented Jul 22, 2022 •
edited
Loading

@fstroth @ai-fast-track @FraPochetti The unit tests are passing on my local machine but not on the CI (OOM Killed).
This looks like a memory issue, this PR adds a lot of tests where a lot of models are created, we had a similar issue last week on our side and needed to run the tests on a machine with more RAM (we have unit tests for the creation of each IceVision models in our code base, similar to what this PR contains).
Is there a way to fix this ?

@ai-fast-track
Copy link
Collaborator

Yes, we do experience that now and then. It's good to know the tests passed on your local machine.

Thank you for your contribution!

@AlexandreBrown
Copy link
Contributor Author

@ai-fast-track It's a pleasure to contribute, we're really enjoying IceVision so far.
Let me know what is left to get it merged, we're in great need of these changes so just let me know and I'll get it done if anything.
Cheers!

fstroth
fstroth approved these changes Jul 23, 2022
View reviewed changes
Copy link
Contributor

@fstroth fstroth left a comment

Choose a reason for hiding this comment

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

Looks good to me, really nice PR. Thank you a lot @AlexandreBrown!

@potipot
Copy link
Contributor

potipot commented Jul 25, 2022

This is really nice, very useful PR.

@AlexandreBrown
Copy link
Contributor Author

Here is a photo showing the tests passing locally, we can also see that the RAM usage was steady (~13GB).
Screenshot from 2022-07-26 08-18-01

@potipot
Copy link
Contributor

potipot commented Jul 26, 2022

If this PR prevents test from passing in the future we should tackle this OOM bug asap. Otherwise gh ci/cd will be misleading

@fstroth fstroth merged commit ca73528 into airctic:master Jul 26, 2022
dnth added a commit to dnth/icevision that referenced this pull request Mar 6, 2024
@dnth @ai-fast-track
@AlexandreBrown @potipot @boscacci @lgvaz @fcakyon
Sync (#2)
25d5ca7 
* install torchtext==0.11.0 (airctic#1119)

* install icevision from master (as default) (airctic#1121)

* Feature/1124 Added Pytorch Lightning Test support for all models (airctic#1125)

* feature/1124 Added Pytorch Lightning Test support for mmdet

* feature/1124 Added Pytorch Lightning Test support for efficientdet

* feature/1124 Added Pytorch Lightning Test support for torchvision

* feature/1124 Added Pytorch Lightning Test support for yolov5

* feature/1124 Renamed shared evaluation method for mmdet

* feature/1124 Renamed shared evaluation method for efficientdet

* feature/1124 Renamed shared evaluation method for torchvision

* feature/1124 Renamed shared evaluation method for yolov5

* feature/1124 Added test for PL test step of efficientdet

* feature/1124 Added test for PL test step of mmdet

* feature/1124 Added test for PL test step of torchvision models

* feature/1124 Added test for PL test step of yolov5

* feature/1124 Updated object detection getting started guide to add PL test

* feature/1124 Updated environment.yml to fix icevision version not found

* feature/1124 Added unit tests for efficientdet lightning model adapter

* feature/1124 Added unit tests for mmdet lightning model adapter

* feature/1124 Added unit tests for torchvision lightning model adapter

* feature/1124 Added unit tests for yolov5 lightning model adapter

* feature/1124 Added unit tests for fastai unet lightning model adapter

* feature/1124 Fixed memory consumption in PL tests

* doc/1116 Added direction on how to import IceVision without wildcard import (airctic#1127)

* Update tests to use newer pytorch lightning (airctic#1133)

* force install pl>=1.7.0

* replace deprecated arguments

* bump minimal pl version from 1.4.6 to 1.5.0

* Export preds to coco annotations (airctic#1132)

* Add a function to export preds to COCO annotations

* Downsample example images to save space; add optional metadata parameter

* Separate 'info' field from the rest of 'addl_info' as a parameter

* Fix unsightly notebook outputs

* Fix slightly errant docstring. TODO: Add type hints

* Black formatting

* Clean up cluttered inference notebook output (airctic#1137)

Co-authored-by: Ubuntu <[email protected]>

* Make COCO exports remember original img resolutions (airctic#1142)

* Make COCO exports remember original img resolutions

* black formatting

* updates sahi dependency

* add support for sahi>=0.11 (airctic#1156)

* add support for sahi>0.11

* pin sahi version

* Added support for 3 bands float32 tiff images (airctic#1138)

* Added support for 3 bands float32 tiff images

* Removed circular dependency
- Bug introduced by airctic#1132
- Credits goes to https://github.com/FraPochetti for finding the issue

* Unit tests micro refactor

---------

Co-authored-by: AI Fast Track <[email protected]>
Co-authored-by: Alexandre Brown <[email protected]>
Co-authored-by: Paweł Potrykus <[email protected]>
Co-authored-by: Robert Boscacci <[email protected]>
Co-authored-by: Ubuntu <[email protected]>
Co-authored-by: Lucas Vazquez <[email protected]>
Co-authored-by: fatih <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fstroth fstroth fstroth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Pytorch Lightning Trainer.test
4 participants
@AlexandreBrown @ai-fast-track @potipot @fstroth