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

Added More Object Detectors #984

Merged
merged 15 commits into from
Jan 23, 2023
Merged

Added More Object Detectors #984

merged 15 commits into from
Jan 23, 2023

Conversation

Niro4
Copy link
Contributor

@Niro4 Niro4 commented Dec 27, 2022

I added RetinaNet and FCOS models to the detection trainer.

@github-actions github-actions bot added testing Continuous integration testing trainers PyTorch Lightning trainers labels Dec 27, 2022
@adamjstewart adamjstewart requested a review from nilsleh December 27, 2022 19:53
adamjstewart
adamjstewart reviewed Dec 27, 2022
View reviewed changes
Copy link
Collaborator

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

Looks great! A bunch of the tests are failing, so we'll need to get those working. I'll let @nilsleh review things in detail since he recently added timm support to all of our other trainers.

.gitignore Outdated Show resolved Hide resolved
torchgeo/trainers/detection.py Outdated Show resolved Hide resolved
@Niro4
Copy link
Contributor Author

Niro4 commented Dec 28, 2022 •
edited
Loading

I resolved the requested changes. I'll look into the failed tests. @nilsleh let me know if there is anything I should change or resolutions to some tests I don't address.

@adamjstewart
Copy link
Collaborator

https://torchgeo.readthedocs.io/en/stable/user/contributing.html has a bunch of tips for running the tests locally and fixing style issues. Let me know if anything isn't clear!

@github-actions github-actions bot added the datasets Geospatial or benchmark datasets label Dec 28, 2022
@Niro4
Copy link
Contributor Author

Niro4 commented Dec 28, 2022

All tests have passed on my local. The datasets tag was added since there was an error thrown by mypy regarding a comment. I removed the comment, and it seemed to resolve the issue.

ashnair1
ashnair1 reviewed Dec 29, 2022
View reviewed changes
Copy link
Collaborator

@ashnair1 ashnair1 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 feature. The following comments should help with resolving mypy and minimum tests. The remaining error is caused by a warning raised by draw_bounding_boxes (fixed by #988).

Once #917 and #988 are merged, all tests should pass.

torchgeo/datasets/idtrees.py Outdated Show resolved Hide resolved
torchgeo/trainers/detection.py Show resolved Hide resolved
tests/trainers/test_detection.py Outdated Show resolved Hide resolved
@adeak adeak mentioned this pull request Dec 31, 2022
Closed
@adamjstewart adamjstewart added this to the 0.4.0 milestone Jan 6, 2023
@adamjstewart
Copy link
Collaborator

@Niro4 #917 is close to being merged. Once that's merged, we can rebase this PR on top of that and get it merged. Can you sign the CLA? That's the only thing I can't do for you 😄

@adamjstewart
Copy link
Collaborator

Update: #917 has been merged

@Niro4
Copy link
Contributor Author

Niro4 commented Jan 23, 2023

@microsoft-github-policy-service agree [company="SWCA Environmental Consultants"]

@adamjstewart
Copy link
Collaborator

Need to remove the [ ] brackets I think

@Niro4
Copy link
Contributor Author

Niro4 commented Jan 23, 2023

@microsoft-github-policy-service agree company="SWCA Environmental Consultants"

@adamjstewart
Copy link
Collaborator

Next step is to rebase. Not sure how comfortable you are with git but let me know if I can help.

@Niro4
Copy link
Contributor Author

Niro4 commented Jan 23, 2023 •
edited
Loading

Thanks! I can handle the rebase. It should be updated shortly.

@github-actions github-actions bot removed the datasets Geospatial or benchmark datasets label Jan 23, 2023
@github-actions github-actions bot added datamodules PyTorch Lightning datamodules datasets Geospatial or benchmark datasets documentation Improvements or additions to documentation models Models and pretrained weights samplers Samplers for indexing datasets scripts Training and evaluation scripts transforms Data augmentation transforms labels Jan 23, 2023
@github-actions github-actions bot removed documentation Improvements or additions to documentation transforms Data augmentation transforms models Models and pretrained weights scripts Training and evaluation scripts datamodules PyTorch Lightning datamodules samplers Samplers for indexing datasets datasets Geospatial or benchmark datasets labels Jan 23, 2023
adamjstewart
adamjstewart requested changes Jan 23, 2023
View reviewed changes
tests/trainers/test_detection.py Outdated Show resolved Hide resolved
torchgeo/trainers/detection.py Outdated Show resolved Hide resolved
torchgeo/trainers/detection.py Outdated Show resolved Hide resolved
@adamjstewart adamjstewart merged commit 3627a32 into microsoft:main Jan 23, 2023
yichiac pushed a commit to yichiac/torchgeo that referenced this pull request Apr 29, 2023
@Niro4 @adamjstewart
Added More Object Detectors (microsoft#984)
87fbe9f 
* Added FCOS and RetinaNet to detection trainer.

* Added detection tests.

* Removed requested lines.

* Test fixes.

* Minor fixes.

* Added FCOS and RetinaNet to detection trainer.

* Added detection tests.

* Removed requested lines.

* Test fixes.

* Minor fixes.

* Added missing import.

* Address review comments

Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashnair1 ashnair1 ashnair1 left review comments

@banesullivan banesullivan banesullivan left review comments

@adeak adeak adeak left review comments

@adamjstewart adamjstewart adamjstewart approved these changes

@nilsleh nilsleh Awaiting requested review from nilsleh

Assignees
No one assigned
Labels
testing Continuous integration testing trainers PyTorch Lightning trainers
Projects
None yet
Milestone
0.4.0
Development

Successfully merging this pull request may close these issues.
5 participants
@Niro4 @adamjstewart @adeak @banesullivan @ashnair1