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 Vaihingen datamodule #853

Merged
merged 17 commits into from
Dec 30, 2022
Merged

Conversation

nilsleh
Copy link
Collaborator

@nilsleh nilsleh commented Oct 16, 2022

This PR closes #851. As suggested, based on the OSCDDatamodule, random patch crops are taken during training. For validation and testing the batch size is fixed to 1 like in OSCDDatamodule but instead of a fixed padding size, each sample is padded to the next larger multiple of 32 for the encoder-decoder architectures.

@github-actions github-actions bot added datamodules PyTorch Lightning datamodules testing Continuous integration testing labels Oct 16, 2022
@adamjstewart adamjstewart added this to the 0.4.0 milestone Oct 16, 2022
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.

I'm sure some of these comments apply to other existing data modules too. Someday I would like to have all public and private methods properly documented. We're using pydocstyle to try to enforce this, but it's broken, so it doesn't complain like it should when methods aren't fully documented.

torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
torchgeo/datamodules/vaihingen.py Outdated Show resolved Hide resolved
@nilsleh
Copy link
Collaborator Author

nilsleh commented Oct 29, 2022

I don't really understand why the minimum test is failing.

@calebrob6
Copy link
Member

The best I can tell is that torch is throwing a UserWarning about the align_corners parameter to an interpolate method. This is called for some reason when we use kornia's RandomCrop. I tried setting it explicitly...

@calebrob6
Copy link
Member

Okay no idea. @adamjstewart any suggestions? Can we bump kornia's minimum version or ignore this warning?

@adamjstewart
Copy link
Collaborator

Since this is a warning (not an error) and the warning has been removed in newer versions of PyTorch, I went with silencing the warning over bumping the minimum version of PyTorch. Latest commit should hopefully get the tests to pass.

@adamjstewart adamjstewart added the backwards-incompatible Changes that are not backwards compatible label Nov 27, 2022
@calebrob6
Copy link
Member

@adamjstewart do you know why rtd is saying "/home/docs/checkouts/readthedocs.org/user_builds/torchgeo/checkouts/853/docs/tutorials/transforms.ipynb:: WARNING: Pygments lexer name 'ipython3' is not known"? Is that why the build failed?

@adamjstewart
Copy link
Collaborator

Should be fixed by #922, just need to rebase.

@calebrob6 calebrob6 force-pushed the datamodule/vaihingen branch from 2b39215 to 6c36f31 Compare December 20, 2022 17:14
@adamjstewart adamjstewart marked this pull request as draft December 20, 2022 17:41
@adamjstewart adamjstewart mentioned this pull request Dec 26, 2022
12 tasks
@github-actions github-actions bot added datasets Geospatial or benchmark datasets scripts Training and evaluation scripts labels Dec 30, 2022
@adamjstewart adamjstewart marked this pull request as ready for review December 30, 2022 04:12
@adamjstewart adamjstewart dismissed isaaccorley’s stale review December 30, 2022 04:13

Requested changes have been implemented

@adamjstewart adamjstewart merged commit a905983 into microsoft:main Dec 30, 2022
yichiac pushed a commit to yichiac/torchgeo that referenced this pull request Apr 29, 2023
* fix datamodule

* requested changes to vaihingen

* data loader

* fix error an clarity

* fix failing test

* fix failing test crop augmentation

* found a bug

* remove same_batch param

* Trying to get minimum tests to pass

* Formatting

* Formatting again

* Update torchgeo/datamodules/vaihingen.py

* Sort imports

* Isort, yousort, we all sort

* Same logic as deepglobe

* More-specific types

* Missing import

Co-authored-by: Caleb Robinson <calebrob6@gmail.com>
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
Labels
backwards-incompatible Changes that are not backwards compatible datamodules PyTorch Lightning datamodules datasets Geospatial or benchmark datasets scripts Training and evaluation scripts testing Continuous integration testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vaihingen datamodule
4 participants