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

Satlas pretrained Swin Transformer v2 models #1358

Merged
merged 21 commits into from
Nov 25, 2023

Conversation

isaaccorley
Copy link
Collaborator

@isaaccorley isaaccorley commented May 23, 2023

Adds Swin Transformer v2 Base pretrained models from the Satlas dataset

@isaaccorley isaaccorley requested a review from adamjstewart May 23, 2023 17:01
@isaaccorley isaaccorley self-assigned this May 23, 2023
@github-actions github-actions bot added models Models and pretrained weights testing Continuous integration testing labels May 23, 2023
@adamjstewart adamjstewart added this to the 0.5.0 milestone May 23, 2023
@adamjstewart
Copy link
Collaborator

Will review after deadline

@github-actions github-actions bot added the documentation Improvements or additions to documentation label May 23, 2023
@calebrob6
Copy link
Member

is the timm swin transformer compatible?

@isaaccorley
Copy link
Collaborator Author

No it's not. The layers are named differently unfortunately.

@isaaccorley
Copy link
Collaborator Author

Wonder if I can map them to each other.

@adamjstewart
Copy link
Collaborator

adamjstewart commented May 24, 2023

If it's not too hard, that would be useful since it would let us use it in our trainers. Wonder if we could add some kind of logic to support both timm and torchvision in our trainers...

@isaaccorley
Copy link
Collaborator Author

It is hard unfortunately.

calebrob6
calebrob6 previously approved these changes Jun 20, 2023
@adamjstewart adamjstewart removed this from the 0.5.0 milestone Sep 28, 2023
@github-actions github-actions bot added the dependencies Packaging and dependencies label Nov 19, 2023
@isaaccorley isaaccorley reopened this Nov 19, 2023
@isaaccorley
Copy link
Collaborator Author

I have no idea why the figure plotting in torchgeo/trainers/detection.py is somehow indirectly not covered via codecov

requirements/min-reqs.old Outdated Show resolved Hide resolved
torchgeo/models/swin.py Outdated Show resolved Hide resolved
torchgeo/models/swin.py Outdated Show resolved Hide resolved
@adamjstewart adamjstewart added this to the 0.6.0 milestone Nov 20, 2023
@adamjstewart
Copy link
Collaborator

Turns out the decrease in test coverage is actually very important!

If you remove the try-except from ObjectDetectionTask, you'll find that there is a bug in our implementation:

FAILED tests/trainers/test_detection.py::TestObjectDetectionTask::test_trainer[True-faster-rcnn-nasa_marine_debris] - ValueError: Boxes need to be in (xmin, ymin, xmax, ymax) format. Use torchvision.ops.box_convert to convert them
FAILED tests/trainers/test_detection.py::TestObjectDetectionTask::test_trainer[True-fcos-nasa_marine_debris] - ValueError: Boxes need to be in (xmin, ymin, xmax, ymax) format. Use torchvision.ops.box_convert to convert them
FAILED tests/trainers/test_detection.py::TestObjectDetectionTask::test_trainer[True-retinanet-nasa_marine_debris] - ValueError: Boxes need to be in (xmin, ymin, xmax, ymax) format. Use torchvision.ops.box_convert to convert them
FAILED tests/trainers/test_detection.py::TestObjectDetectionTask::test_no_rgb[True] - ValueError

These were being masked by our try-except. The code works fine for torchvision 0.13 but breaks for any newer version. We didn't notice this because the combined coverage didn't change. From what I can tell, this has always been broken...

@adamjstewart
Copy link
Collaborator

Can you try to fix that in a separate PR so we can backport it to 0.5.2? Or one of us can do it, just let me know if you're busy.

@isaaccorley isaaccorley changed the title add Satlas pretrained models Satlas pretrained Swin Transformer v2 models Nov 20, 2023
@isaaccorley
Copy link
Collaborator Author

Can you try to fix that in a separate PR so we can backport it to 0.5.2? Or one of us can do it, just let me know if you're busy.

I'll push a PR here shortly

adamjstewart
adamjstewart previously approved these changes Nov 21, 2023
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.

Can merge as soon as coverage issue is fixed.

@adamjstewart adamjstewart enabled auto-merge (squash) November 25, 2023 20:36
@adamjstewart adamjstewart merged commit 8c24c80 into microsoft:main Nov 25, 2023
21 checks passed
@isaaccorley isaaccorley deleted the models/satlas branch December 6, 2023 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Packaging and dependencies documentation Improvements or additions to documentation models Models and pretrained weights testing Continuous integration testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants