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

Add Inria datamodule #498

Merged
merged 26 commits into from
May 22, 2022
Merged

Add Inria datamodule #498

merged 26 commits into from
May 22, 2022

Conversation

ashnair1
Copy link
Collaborator

@ashnair1 ashnair1 commented Apr 5, 2022 •
edited
Loading

  • Add Inria datamodule
  • Add tests and update test data

Prediction utilities
Added predict.py. Allows running inference on test sets / directory of images. Requires kornia>0.6.4 for fixed CombineTensorPatches.

@github-actions github-actions bot added datamodules PyTorch Lightning datamodules datasets Geospatial or benchmark datasets documentation Improvements or additions to documentation testing Continuous integration testing labels Apr 5, 2022
adamjstewart
adamjstewart reviewed Apr 5, 2022
View reviewed changes
tests/datamodules/test_inria.py Show resolved Hide resolved
predict.py Outdated Show resolved Hide resolved
conf/inria.yaml
ignore_zeros: True # class 0 not used for scoring
datamodule:
root_dir: "data/inria"
batch_size: 2
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is pretty small... Note that conf/inria.yaml should contain the optimal hyperparameters while tests/conf/inria.yaml should contain the bare minimum to get the tests to run.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added tests/conf/inria.yaml

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that the small values required to make the tests run fast are in tests/conf/inria.yaml, should these values be increased? I guess it requires a hyperparam tuning to determine what the best hyperparams are.

setup.cfg Show resolved Hide resolved
torchgeo/datamodules/inria.py Show resolved Hide resolved
torchgeo/datasets/utils.py Outdated Show resolved Hide resolved
@adamjstewart adamjstewart added this to the 0.3.0 milestone Apr 5, 2022
@calebrob6
Copy link
Member

calebrob6 commented Apr 20, 2022 •
edited
Loading

@ashnair1 for the predict PR are you planning a predict.py script that can run a model over a scene of imagery?

Nevermind, I see that this was discussed earlier.

calebrob6
calebrob6 previously approved these changes Apr 20, 2022
View reviewed changes
@ashnair1
Copy link
Collaborator Author

ashnair1 commented May 19, 2022 •
edited
Loading

Docs are failing due to the recent deprecation of core/lightning.py in favor of core/module.py in pytorch-lightning. Refer Lightning-AI/pytorch-lightning#12740.

Edit: Docs issue solved.

@ashnair1
Copy link
Collaborator Author

@adamjstewart @calebrob6 @isaaccorley

Anything left to address in this PR?

adamjstewart
adamjstewart reviewed May 22, 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.

Thanks, I think this is really close, just a few minor comments!

Also, I want to apologize for taking so long to review this. I just finished all of my final projects and exams for the semester and started an internship, so it's been a really busy month. I'm hoping to spend more of my weekends on TorchGeo over the summer so hopefully I'll be more responsive from now on!

setup.cfg Outdated Show resolved Hide resolved
torchgeo/datamodules/inria.py Outdated Show resolved Hide resolved
torchgeo/datamodules/inria.py Outdated Show resolved Hide resolved
torchgeo/datamodules/inria.py Outdated Show resolved Hide resolved
torchgeo/datamodules/inria.py Outdated Show resolved Hide resolved
@ashnair1
Copy link
Collaborator Author

Thanks, I think this is really close, just a few minor comments!

Also, I want to apologize for taking so long to review this. I just finished all of my final projects and exams for the semester and started an internship, so it's been a really busy month. I'm hoping to spend more of my weekends on TorchGeo over the summer so hopefully I'll be more responsive from now on!

No worries. You're doing plenty as is. Thanks.

Got back to this PR because a colleague wanted to try out a building detector on their own data. Honestly my original motivation was the same. Train a model on this dataset and add utilities for running inference on geotiffs of arbitrary size by leveraging kornia. Got sidetracked by other projects but I'm hoping to get this done soon.

@adamjstewart adamjstewart merged commit d06b103 into microsoft:main May 22, 2022
@ashnair1 ashnair1 deleted the inria-dm branch May 23, 2022 10:49
remtav pushed a commit to remtav/torchgeo that referenced this pull request May 26, 2022
@ashnair1 @remtav
Add Inria datamodule (microsoft#498)
eeac9f7 
* Add Inria Datamodule

* Fix up

* Add predict.py

* Integrate kornia fns for extracting & combining

Requires kornia/kornia#1558 to be merged

* transform creates problem when calculating metrics

* Update

* Use dict.get

* Add tests & update test data

* Add Inria datamodule to docs

* Reduce test data size

* Datamodules always have predict_dataloader

* Remove comments

* Update predict.py

* Add PredictDataset

* Fix tests

* Update inria.yaml

* Clarify predict_on doc

* Refactor

* Update min kornia

* Update inria.yaml

* Remove predict utilities

* Trainer fix

* Use kornia's compute_padding

* kornia docfix

* Use stable docs

* Fixes
This was referenced Jul 9, 2022
Merged
Merged
yichiac pushed a commit to yichiac/torchgeo that referenced this pull request Apr 29, 2023
@ashnair1
Add Inria datamodule (microsoft#498)
db34d0f 
* Add Inria Datamodule

* Fix up

* Add predict.py

* Integrate kornia fns for extracting & combining

Requires kornia/kornia#1558 to be merged

* transform creates problem when calculating metrics

* Update

* Use dict.get

* Add tests & update test data

* Add Inria datamodule to docs

* Reduce test data size

* Datamodules always have predict_dataloader

* Remove comments

* Update predict.py

* Add PredictDataset

* Fix tests

* Update inria.yaml

* Clarify predict_on doc

* Refactor

* Update min kornia

* Update inria.yaml

* Remove predict utilities

* Trainer fix

* Use kornia's compute_padding

* kornia docfix

* Use stable docs

* Fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamjstewart adamjstewart adamjstewart approved these changes

@calebrob6 calebrob6 calebrob6 left review comments

Assignees
No one assigned
Labels
datamodules PyTorch Lightning datamodules datasets Geospatial or benchmark datasets documentation Improvements or additions to documentation testing Continuous integration testing
Projects
None yet
Milestone
0.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ashnair1 @calebrob6 @adamjstewart