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 multiple versions of the So2Sat dataset #1283

Merged
merged 22 commits into from
Apr 25, 2023
Merged

Add multiple versions of the So2Sat dataset #1283

merged 22 commits into from
Apr 25, 2023

Conversation

calebrob6
Copy link
Member

@calebrob6 calebrob6 commented Apr 24, 2023 •
edited
Loading

@github-actions github-actions bot added the datasets Geospatial or benchmark datasets label Apr 24, 2023
@github-actions github-actions bot added the testing Continuous integration testing label Apr 24, 2023
@github-actions github-actions bot added the datamodules PyTorch Lightning datamodules label Apr 24, 2023
@calebrob6 calebrob6 marked this pull request as ready for review April 25, 2023 00:37
@adamjstewart adamjstewart added this to the 0.5.0 milestone Apr 25, 2023
calebrob6
calebrob6 commented Apr 25, 2023
View reviewed changes
tests/data/so2sat/data.py Show resolved Hide resolved
tests/data/so2sat/data.py Show resolved Hide resolved
tests/data/so2sat/data.py Show resolved Hide resolved
tests/data/so2sat/data.py Show resolved Hide resolved
torchgeo/datamodules/so2sat.py Outdated Show resolved Hide resolved
torchgeo/datamodules/so2sat.py Show resolved Hide resolved
@github-actions github-actions bot added the dependencies Packaging and dependencies label Apr 25, 2023
@calebrob6
Copy link
Member Author

calebrob6 commented Apr 25, 2023 •
edited
Loading

Tests are currently failing because our min version of pytorch is 1.12.0, and percentages aren't supported by torch.utils.data.random_split until 1.13.0. I bumped the min version because in our deprecated torchgeo.datamodules.utils.dataset_split we recommend using random_split with percentages. I use the 1.12.0 way of specifying train and val set sizes so that we can update dependencies in a different PR if we want.

@github-actions github-actions bot removed the dependencies Packaging and dependencies label Apr 25, 2023
adamjstewart
adamjstewart reviewed Apr 25, 2023
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.

Not sure if @nilsleh @wangyi111 want to review since the dataset is from their lab.

torchgeo/datamodules/so2sat.py Outdated Show resolved Hide resolved
torchgeo/datasets/so2sat.py Show resolved Hide resolved
torchgeo/datamodules/so2sat.py Outdated Show resolved Hide resolved
@adamjstewart
Copy link
Collaborator

Can you rebase/merge? I added EOL chars to all config files

@calebrob6
Copy link
Member Author

Done

adamjstewart
adamjstewart requested changes Apr 25, 2023
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.

Bunch of minor formatting changes, plus parameter rename and doc fix

tests/conf/so2sat_rgb.yaml Outdated Show resolved Hide resolved
torchgeo/datamodules/so2sat.py Outdated Show resolved Hide resolved
torchgeo/datasets/so2sat.py Outdated Show resolved Hide resolved
torchgeo/datasets/so2sat.py Outdated Show resolved Hide resolved
torchgeo/datasets/so2sat.py Outdated Show resolved Hide resolved
torchgeo/datasets/so2sat.py Outdated Show resolved Hide resolved
torchgeo/datasets/so2sat.py Outdated Show resolved Hide resolved
adamjstewart
adamjstewart approved these changes Apr 25, 2023
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.

LGTM, assuming docs look good

@calebrob6 calebrob6 enabled auto-merge (squash) April 25, 2023 21:51
@calebrob6
Copy link
Member Author

Yep I checked in the last build

@calebrob6 calebrob6 merged commit a0ae00c into main Apr 25, 2023
@calebrob6 calebrob6 deleted the datasets/so2sat branch April 25, 2023 22:03
@adamjstewart adamjstewart mentioned this pull request Apr 28, 2023
Closed
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

Assignees
No one assigned
Labels
datamodules PyTorch Lightning datamodules datasets Geospatial or benchmark datasets testing Continuous integration testing
Projects
None yet
Milestone
0.5.0
Development

Successfully merging this pull request may close these issues.

Implement the "third version" of the So2Sat dataset
2 participants
@calebrob6 @adamjstewart